Repository navigation
Rust port: why the rewrite is incoherent, and the two triggers that would make the hybrid worth revisiting (from #775) #779
Replies: 2 comments
|
Measured on real runners now, and the number I quoted here needs correcting: The structure of the finding held on all three platforms: One thing I got wrong here, worth flagging because it was the actionable half: I suggested this was a function-local import away. It is not. Filed with the full tables, the import graph and a repro as #780. It does not change this thread's conclusion — the startup gate for a Rust port stays unmet, and the remaining cost is Soup's own pydantic model tree, which clap would not touch. |
|
Closing this as resolved — measured rather than argued, and nothing here asks anything of Soup. Soup already gets Rust where Rust pays. Resolving Also checked: swapping stdlib Your two gates stand unchanged. Reopen if either fires. |
Uh oh!
There was an error while loading. Please reload this page.
Moved here from #775 at @MakazhanAlpamys' request — "Please move it to a Discussion; converting it to issues now would create work with no trigger." Nothing is being asked for. The point of writing it down is that the analysis is cheaper to reopen than to redo, and the triggers that would justify reopening it are worth naming while they are still absent.
Position up front: a Rust rewrite of Soup is incoherent, and even the hybrid track has no trigger today. The measurement in the last section — which I ran to test the one gate that was cited in #775 — argues against the Rust track and for a plain Python fix instead.
Why the rewrite is not on the table
Soup's value is that it wraps the Python ML stack: torch, transformers, peft, trl, bitsandbytes, accelerate, mlx. Porting the trainer surface (SFT plus the preference trainers, QLoRA, FSDP) means either reimplementing that stack on
candle/burn, which do not currently cover it, or paying FFI costs stacked on top of the trl/transformers API churn this project already absorbs. Both are worse than what exists.So the only version worth discussing is a hybrid: leave training in Python, move some non-training layer to Rust.
The hybrid layers, honestly ranked
config/schema.py(7,053 lines)datasetsconfig/schema.pyinverts on inspection. It is the largest single module and the most mechanical, which is what makes serde look attractive — but it is also the surface that tracks trl/transformers kwargs, i.e. the highest-churn file in the repo. Porting it would move the churn across an FFI boundary and duplicate the source of truth CONTRIBUTING names as single ("These models are the single source of truth for valid fields and defaults"). It is the worst candidate, not the best.The two triggers
@MakazhanAlpamys named both in #775, and they are the right gates:
Cargo.tomland no maturin inpyproject.toml, so a hybrid would also introduce a build toolchain and a wheel matrix to a currently pure-Python distribution.If neither fires, this stays closed. If (1) fires, the order is CLI shell first, then data tooling; config stays in pydantic.
Testing the startup gate — the answer is a Python fix, not a Rust one
#775 cites
tests/test_cli_startup_is_light.pyas governing startup on the Python side. Read closely, that guard pins something narrower than "startup is fine": it asserts that heavy ML modules do not leak into import time (torch,transformers,mlx, …), acrosssoup_cli.cliand 50+ command modules. It does not assert a wall-clock budget, so the project's own modules can get expensive without the guard noticing. So I measured.-X importtime, largest cumulative subtrees:Caveats, because these numbers decide the argument. One machine, Windows, Python 3.12,
python -m soup_clirather than the console script, in auv-provisioned environment; 5 runs, min and median reported. Windows process startup is slower than Linux, so the absolute figures are an upper bound and are not a CI-comparable measurement.-X importtimeinflates totals (its own tree sums to ~4.1 s against a ~1.2–1.7 s wall clock), so only the relative shares above should be read, not those durations. Someone on Linux posting the same two commands would sharpen this considerably.What survives the caveats:
soup versioncosts roughly 1.3 s against an ~90 ms interpreter floor, and the largest identified share is Soup importing its own 7,053-line pydantic schema throughcommands.autopilot— not typer, not rich, and no ML dependency. The heavy-import guard is doing its job; this cost is below its threshold by design.That points at deferring
soup_cli.autopilotbehind a function-local import in the same style CONTRIBUTING already prescribes for the ML stack, not at clap. A rewrite that shaved the typer/rich share whileconfig.schemastill loaded eagerly would spend a language migration on the smaller half of the problem.I have not filed that as an issue — it is a measurement from one Windows box, and it belongs to whoever wants to confirm it on Linux CI first. Happy to open one with a startup-budget test alongside the fix if that is wanted; equally happy for it to stay here.
Refs #775.
All reactions