Skip to content

fix: accept embeddings= alias and warn on unknown Llama kwargs#2345

Open
Solaris-star wants to merge 1 commit into
abetlen:mainfrom
Solaris-star:fix/2210-embeddings-kwarg-alias
Open

fix: accept embeddings= alias and warn on unknown Llama kwargs#2345
Solaris-star wants to merge 1 commit into
abetlen:mainfrom
Solaris-star:fix/2210-embeddings-kwarg-alias

Conversation

@Solaris-star

Copy link
Copy Markdown

Summary

Llama() historically takes embedding=True. Callers that pass the plural embeddings=True (or other typos) hit a bare **kwargs sink: the flag is silently ignored, then .embed() fails later with a misleading RuntimeError.

Changes

  • Accept embeddings: Optional[bool] = None as an alias of embedding
  • Reject conflicting embedding / embeddings values
  • Emit a UserWarning for unexpected kwargs instead of swallowing them silently
  • Clarify the .embed() error message to mention both spellings
  • Add a unit test that does not require a real GGUF model

Fixes #2210

Test plan

  • Logic unit-tested offline for alias / conflict / unknown-kwargs paths
  • tests/test_llama.py::test_embeddings_kwarg_alias_and_unknown_kwargs added
  • Full suite needs a GGUF fixture (CI)

The constructor historically used embedding=True. Callers that pass
embeddings=True (plural) were silently swallowed by **kwargs, so
embed() later raised a confusing RuntimeError. Accept embeddings as an
alias, reject conflicting values, and warn on unexpected kwargs.

Fixes abetlen#2210

Signed-off-by: Solaris-star <820622658@qq.com>
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.

Llama() silently accepts and discards embedding kwarg; .embed() then raises confusingly

1 participant