Aller au contenu
login
arrow_backRetour aux issues
MakazhanAlpamys/Soup #478

CI never exercises the declared transformers floor, so a >=4.56-only kwarg passes 12/12

ecoDébutant enhancement help wanted

descriptionDescription

Found while reviewing #471, which was caught by hand rather than by CI. The PR is fine; the gap is ours. ## The gap `pyproject.toml` declares: ``` transformers>=4.36.0,<5.0.0 ``` CI installs with `pip install -e ".[dev]"`, which resolves to the **newest** satisfying version (4.57.x today). So the floor of our own declared range is **never executed**. A change that works on 4.57 and is a hard `TypeError` on 4.55 goes green on all 12 cells. That is not hypothetical — it is exactly what happened in #471: ``` transformers 4.55.4 dtype='auto' -> TypeError: __init__() got an unexpected keyword argument 'dtype' transformers 4.57.6 dtype='auto' -> loaded ``` `dtype=` is the >= 4.56 rename of `torch_dtype=`. Twenty minor versions inside our declared support would have failed at model load, before step 0, on text/vision/audio/QLoRA alike. ## Why it matters more than a version skew usually would **pip does not upgrade an already-satisfying dependency.** Someone sitting on 4.55.4 who runs `pip install soup-cli[train]` keeps 4.55.4 — our pin is satisfied. That is the older-image Colab/Kaggle population, i.e. the same fleet #385/#387 was about. `grep -rn "4.56" .` returns nothing: no test, no comment, no constraint anywhere records that a floor exists to respect. ## Two things this is NOT - Not an argument for raising the floor. Deciding `>=4.56` is a dependency-policy call with its own cost (it drops those users rather than supporting them). - Not solved by a lockfile. `soup env lock` records what a user *has*; this is about what our pin *promises*. ## Acceptance - [ ] Something exercises the declared floor. Cheapest credible form is one CI cell that installs `transformers==4.36.0` (or the lowest version that actually resolves with our other pins) and runs a targeted subset — model-construction paths, not the full 18k suite. - [ ] It must FAIL on a `dtype=`-style regression. Verify by writing the failure first: put `dtype=` back at one `from_pretrained` site and confirm the new cell goes red while the existing 12 stay green. A floor cell that cannot fail is worse than none, because it looks like coverage. - [ ] If a true floor run is impractical (older transformers may not resolve against current torch/peft/trl), the fallback is a static guard: a test asserting no `from_pretrained` call site uses a kwarg introduced after our floor. Narrower, but it would have caught #471. ## Related The same class already bit us the other way in #326 (`max_prompt_length` removed by trl in stages, a version table wrong twice) — the answer there was to probe **capabilities** rather than versions. Worth considering here: `"torch_dtype" if "dtype" not in signature else "dtype"` is uglier than a floor cell but has no version table to get wrong.
codeOuvre sur GitHub