Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LanguageBind models normalization parameters parse #1031

Merged
merged 15 commits into from
Nov 4, 2024

Conversation

wanliAlex
Copy link
Collaborator

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    bug fix

  • What is the current behavior? (You can also link to an open issue here)
    we are having an issue if we call vectorise on a language bind model. we see:

    on_start(_config)
  File "/app/src/marqo/tensor_search/on_start_script.py", line 45, in on_start
    thing_to_start.run()
  File "/app/src/marqo/tensor_search/on_start_script.py", line 180, in run
    _ = _preload_model(model=model, content=test_string, device=device)
  File "/app/src/marqo/tensor_search/on_start_script.py", line 278, in _preload_model
    _ = vectorise(
  File "/app/src/marqo/s2_inference/s2_inference.py", line 72, in vectorise
    return _vectorise_without_cache(model_cache_key, content, normalize_embeddings, modality, media_download_headers,
  File "/app/src/marqo/s2_inference/s2_inference.py", line 135, in _vectorise_without_cache
    return _encode_without_cache(model_cache_key, content, normalize_embeddings, modality, media_download_headers, **kwargs)
  File "/app/src/marqo/s2_inference/s2_inference.py", line 146, in _encode_without_cache
    vectorised = model.encode(
  File "/app/src/marqo/s2_inference/multimodal_model_load.py", line 116, in encode
    print(**kwargs)
TypeError: 'normalize' is an invalid keyword argument for print()
  • What is the new behavior (if this is a feature change)?
    we fix the bug

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    no

  • Have unit tests been run against this PR? (Has there also been any additional testing?)

no

  • Related Python client changes (link commit/PR here)
    no

  • Related documentation changes (link commit/PR here)
    no

  • Other information:
    no

  • Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added / updated (for bug fixes / features)

RaynorChavez
RaynorChavez previously approved these changes Nov 1, 2024
@wanliAlex wanliAlex requested a review from papa99do November 1, 2024 06:51
farshidz
farshidz previously approved these changes Nov 4, 2024
tests/s2_inference/test_large_model_encoding.py Outdated Show resolved Hide resolved
@wanliAlex wanliAlex merged commit d78a563 into mainline Nov 4, 2024
9 checks passed
@wanliAlex wanliAlex deleted the li/fix-language-bind-warm-up branch November 4, 2024 03:39
vicilliar pushed a commit that referenced this pull request Nov 29, 2024
…erated normalised embeddings bug (#1031)

This PR fixes two bugs:
- 1. An internal error is raised if we pass a string to the vectorise function for Languagebind models;
- 2. Languagebind models always generate normalised embeddings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants