Conversation
…coder Some HuggingFace models (e.g. nomic-ai/nomic-embed-text-v1) require trust_remote_code=True to load, which previously required patching pyserini or accepting a blocking Y/N terminal prompt every run. Threads an optional trust_remote_code=False parameter through to both AutoModel.from_pretrained() and load_auto_tokenizer() (which already forwards **kwargs to AutoTokenizer.from_pretrained(), so no change needed there). Defaults to False, matching current behavior.
AutoDocumentEncoder/AutoQueryEncoder gained trust_remote_code in the prior commit, but FaissSearcher/BinaryDenseFaissSearcher never forwarded it - the exact "encode queries and search a precomputed index" workflow issue castorini#2207 describes, so that use case stayed blocked even after the encoder classes were fixed. Thread trust_remote_code through FaissSearcher.__init__, from_prebuilt_index, the _init_encoder_from_str dispatcher, and BinaryDenseFaissSearcher.__init__. All additions are appended after existing params with a False default, so no caller breaks (checked every call site, including the one that already passes 3 positional args to from_prebuilt_index). Add a mock-based unit test on _init_encoder_from_str mirroring the existing SPLADE test's patch-and-assert pattern, since this repo has no CI to lean on and building the Anserini fatjar needed to import pyserini.search for real is out of scope here (same constraint noted on castorini#2644). AI-assisted (Claude Code); verified with a local test run before committing (see CONTRIBUTION_NOTES.md, not part of this diff, for how).
Contributor
Author
|
Extended this to also cover `FaissSearcher`/`BinaryDenseFaissSearcher`/`from_prebuilt_index` (via `_init_encoder_from_str`) — the first commit only touched `AutoDocumentEncoder`/`AutoQueryEncoder` directly, but the issue's own motivating use case ("encoding queries and searching from a precomputed index") normally goes through `FaissSearcher`, which didn't have a way to pass `trust_remote_code` through. It does now, same append-only/default-False pattern as before. Added a mock-based test for the new dispatch logic. Updated the PR description above to match. Note AI-assisted (Claude Code) — should have flagged this in the original PR description too, apologies for the omission there. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2207
What
Adds an optional
trust_remote_code: bool = Falseparameter toAutoDocumentEncoderandAutoQueryEncoder, threaded through to bothAutoModel.from_pretrained()and the tokenizer loader, and further threaded throughFaissSearcher/BinaryDenseFaissSearcherso it also reaches the "search a precomputed index" workflow the issue itself describes. This lets models that require custom code on the Hub (e.g.nomic-ai/nomic-embed-text-v1, named in the issue) load without a blocking Y/N terminal prompt or needing to patch pyserini locally.Scope
Scoped to the two
Auto*Encoderclasses named in the issue (the genericAutoModel.from_pretrained(...)-based wrappers — model-specific encoders elsewhere inpyserini/encode/like uniCOIL/SPLADE/DPR load fixed, known architectures and don't need this knob), plusFaissSearcher/BinaryDenseFaissSearcher/from_prebuilt_index— those are the actual entry points for "encoding queries and searching from a precomputed index" that the issue names as the motivating use case, and the fix wasn't reachable from there in the first commit.load_auto_tokenizer()in_base.pyalready forwards**kwargstoAutoTokenizer.from_pretrained(), so no change was needed there beyond passingtrust_remote_codethrough as a kwarg.All new parameters default to
Falseand are appended after existing params, so existing behavior/call sites are unchanged for everyone not opting in (checked every call site, including one that already passes 3 positional args tofrom_prebuilt_index).Not included (happy to add if wanted)
Didn't wire this into the CLI scripts (
scripts/encode_queries.py,pyserini/encode/__main__.py,pyserini/encode/query.py's CLI driver) — no--trust-remote-codeflag there yet. Let me know if that'd be useful too.Validation
Checked every call site of
AutoDocumentEncoder/AutoQueryEncoder/FaissSearcherin the repo — all keyword-arg based (or safely appended after existing params), so this is fully backward compatible.Added
tests/base/search/test_faiss_searcher_trust_remote_code.py, a mock-based unit test on_init_encoder_from_strmirroring the existing SPLADE test's patch-and-assert pattern — no network or model download needed.Couldn't run the full suite or even import
pyserini.searchfor real: this repo needs a built/downloaded Anserini fatjar just to import that package (same wall as #2644), and there's no CI here to compare against either (no.github/workflowsin this repo). Verified the new tests pass using a local, uncommitted stub of the JVM bridge (pyserini.pyclass) instead — confirms the logic, not a real end-to-end run.Note
AI-assisted (Claude Code); verified against the source and a local test run before each push.