Repository navigation
fix: resolve BART and T5 refiner models in get_refiner - #254
Merged
ignorejjj merged 1 commit intoOct 12, 2026
Merged
Conversation
get_refiner lower-cases the model architecture name and then checks it
for 'T5' and 'Bart', which can never match. With the configuration from
the refiner docs (refiner_name: abstractive plus a BART or T5 model path)
no branch matches and get_refiner raises ValueError('No implementation!').
Only names containing 'recomp' reached the abstractive refiner.
Compare against lower-case 't5' and 'bart'.
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
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.
Summary
get_refinerlower-cases the architecture name read from the model config, but then compares it against mixed-case strings:So the abstractive refiner can only be selected through the
"recomp" in refiner_namebranch. The configuration described in the refiner docs (docs/zh-cn/component/refiner.md:refiner_name"通常设置为abstractive即可" with a BART/T5refiner_model_path) matches no branch and fails:(Reproduced on
mainat0e57de4with localconfig.jsonfiles and the refiner class stubbed, so no model download is needed.)Change
't5'/'bart'. Every other branch is unchanged, and thebertcheck that comes before it already used lower case.tests/test_get_refiner.py(force-added like the other tests, sincetests/*is ignored). It writes minimal BART / T5 / BERTconfig.jsonfiles to a temp dir and stubs the refiner classes. It checks thatabstractiveresolves toAbstractiveRecompRefinerfor BART and T5, and that a BERT model still resolves toExtractiveRefiner.Tests
python -m pytest tests/test_get_refiner.py: 2 passed. Without the source change, both seq2seq subtests fail withValueError: No implementation!.python -m pytest tests: 28 passed. The 2 subtests oftest_retriever_cache.py::test_single_search_with_no_cached_resultsfail identically onmainin my environment (operator torchvision::nms does not exist, a local torch/torchvision mismatch) and are unrelated.