Skip to content

fix: resolve BART and T5 refiner models in get_refiner - #254

Merged
ignorejjj merged 1 commit into
RUC-NLPIR:mainfrom
kabishou11:fix/refiner-seq2seq-arch-match
Oct 12, 2026
Merged

ignorejjj merged 1 commit into
RUC-NLPIR:mainfrom
kabishou11:fix/refiner-seq2seq-arch-match

Conversation

@kabishou11

Copy link
Copy Markdown
Contributor

Summary

get_refiner lower-cases the architecture name read from the model config, but then compares it against mixed-case strings:

arch = model_config.architectures[0].lower()      # "bartforconditionalgeneration"
...
elif 'T5' in arch or 'Bart' in arch:               # never true
    refiner_class = "AbstractiveRecompRefiner"

So the abstractive refiner can only be selected through the "recomp" in refiner_name branch. The configuration described in the refiner docs (docs/zh-cn/component/refiner.md: refiner_name "通常设置为abstractive即可" with a BART/T5 refiner_model_path) matches no branch and fails:

refiner_name='abstractive', model=BartForConditionalGeneration -> ValueError: No implementation!
refiner_name='abstractive', model=T5ForConditionalGeneration   -> ValueError: No implementation!

(Reproduced on main at 0e57de4 with local config.json files and the refiner class stubbed, so no model download is needed.)

Change

  • Compare against lower-case 't5' / 'bart'. Every other branch is unchanged, and the bert check that comes before it already used lower case.
  • Added tests/test_get_refiner.py (force-added like the other tests, since tests/* is ignored). It writes minimal BART / T5 / BERT config.json files to a temp dir and stubs the refiner classes. It checks that abstractive resolves to AbstractiveRecompRefiner for BART and T5, and that a BERT model still resolves to ExtractiveRefiner.

Tests

  • python -m pytest tests/test_get_refiner.py: 2 passed. Without the source change, both seq2seq subtests fail with ValueError: No implementation!.
  • python -m pytest tests: 28 passed. The 2 subtests of test_retriever_cache.py::test_single_search_with_no_cached_results fail identically on main in my environment (operator torchvision::nms does not exist, a local torch/torchvision mismatch) and are unrelated.

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'.
@ignorejjj
ignorejjj merged commit 00a6c5e into RUC-NLPIR:main Oct 12, 2026
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.

2 participants