Repository navigation
[Deps] Require laya 0.3.9, lock 0.3.20, and drop the local weight-init skip - #37
Conversation
|
@cacheline999 could you add a Demo / evidence section following the self-review guidance? The CPU/MPS measurements already answer the right questions; the missing piece is a reproducible artifact behind the tables. Please link the comparison script and sanitized per-run outputs for the same ticket inputs on baseline/head: model loading, one- and six-question answers, and the default MPS setting versus LAYA_MPS_AMP_MIN_ROWS=5. Include the exact source/checkpoint revisions and invocation, and retain the two changed multilingual decisions in the override results. Keep the existing cold-load/background-load notes and untested configurations explicit. Existing logs or a terminal trace are enough; no video or broader hardware sweep is needed. Please use safe sample data and redact credentials/private paths from anything shared. |
ccd7903 to
82f7f21
Compare
|
Added, see the Demo / evidence section. |
can you record a video if possible and attach it under this PR? we will post blogs based on your PRs. BTW, which inference engine did you use? have you ever tried system1-omni repo: https://github.com/ThinkFlowLab/system1-omni The current CPU/MPS comparison calls in-process Laya. System1-Omni already supports Laya serving, and #35 provides the |
5fb5fe3 to
13d9613
Compare
laya 0.3.9 builds the encoder under transformers' no_init_weights inside laya.load and loads the checkpoint with strict=True, so the without_weight_init() wrapper from ThinkFlowLab#22 and its test stubs are no longer needed. The lock moves laya from 0.3.5 to 0.3.9, the first release with the skip; later releases change MPS precision and are left for their own bump. The system_one config-error test pins the version lookup, so it passes with the laya extra installed as well as on the core install. Closes ThinkFlowLab#28
uv lock --upgrade-package laya, as ThinkFlowLab#28 asks, pinned to 0.3.20, the release system1-omni's worker runs. Only the laya entry of the lock changes. From 0.3.10 laya runs a request of five or more questions in fp16 on MPS. Against fp32 that moved probabilities by up to 0.05 on the multilingual checkpoint and flipped 2 of 180 decisions, and on an M1 Pro it was slower for short states. LayaModel.from_env now raises the agent's mps_amp_min_rows so such requests stay in fp32, unless LAYA_MPS_AMP_MIN_ROWS is set. With that, answers match main on CPU and MPS for both cached checkpoints. Also tests the 'laya unknown' branch, which the pinned version lookup had stopped covering.
Laya reads a value it cannot parse as its default of 5, which runs requests of five or more questions in fp16 on MPS. from_env saw the variable set and left that in place, so a mistyped value meant to keep fp32 did the opposite. It is now a configuration error, raised before the checkpoint loads.
docs/results/laya-upgrade: run.sh checks out two commits, installs each one's lock, runs --model laya's backend over the 30 ticket-router tickets (one choice, one noul and six questions) on both checkpoints and devices, adds the LAYA_MPS_AMP_MIN_ROWS=5 and no-skip controls, and prints the comparison. The README gives the steps. The recorded run's outputs are attached to ThinkFlowLab#37 rather than committed, per the artifact hygiene guide.
13d9613 to
b6edc84
Compare
|
The comparison in the description uses in-process Laya. The video runs the same three tickets both ways: pr37-demo.mp4Both runs' traces and the 28 comparison records: pr37-evidence.zip |
hsliuustc0106
left a comment
There was a problem hiding this comment.
This PR moves the laya extra from >=0.3.4 to >=0.3.9, locks laya 0.3.5 → 0.3.20, deletes the local without_weight_init() wrapper (laya skips the throwaway random init itself from 0.3.9), and adds a guard so --model laya keeps MPS requests in fp32 (raising agent.mps_amp_min_rows to 10**9) unless the user sets laya's own LAYA_MPS_AMP_MIN_ROWS, which is validated as a whole number before the checkpoint loads. Note on scope: the local main (e7b7d2b) is stale relative to this PR's real base — #36 and #50 are merged upstream (merge commits 22e685b, 77f5515), so the true PR diff (merge-base 77f5515) is 12 files; the s1a/probe.py, wire.py, jev.py and timeout changes visible in a naive git diff main...HEAD belong to those merged PRs, not #37.
1. Lock and constraint are consistent; nothing else moved [P3-none]
pyproject.toml:43 sets laya>=0.3.9; uv.lock pins 0.3.20 with matching requires-dist. The lock diff contains only the laya package entry (version, URLs, hashes) and the specifier — no silent bumps of torch/transformers/etc. I downloaded the real 0.3.9 and 0.3.20 wheels from PyPI and verified the floors: 0.3.9 already calls no_init_weights() inside its own load (laya/agent.py:171-173,250) and has no mps_amp_min_rows; 0.3.20 sets self.mps_amp_min_rows = _mps_amp_min_rows() (agent.py:394) from max(1, int(env)) with a silent fallback to default 5 — exactly the silent-fp16 hazard the new validation guards against, and the reason the error must fire before laya.load (tested: loads == []).
2. Weight-init skip removal is safe and complete [P3-none]
without_weight_init() and its importlib/contextlib imports are gone (s1a/decision_models/laya.py), the three stub tests and the factory-test stub are deleted with it, and the only remaining reference is docs/results/laya-upgrade/laya_ab.py:68, which monkeypatches it for the baseline (main) checkout only — dead attribute on head, harmless. The hasattr guard makes a 0.3.9 install a correct no-op, and that path is tested.
3. Env docs are consistent [P3-none]
.env.example, the docs/configuration.md table row and the CHANGELOG entry all describe LAYA_MPS_AMP_MIN_ROWS the same way (unset → fp32; 5 is laya's default; non-whole-number is a config error), matching the code. The CHANGELOG drops the old machine-specific "about 3 s" claim as its own commit promised.
4. Upcoming textual conflicts with #9, which has since merged to main [P2] — #9 landed on main (9790b52) after this branch's last main sync, so the overlaps below are now real: this branch and #9 both (a) insert constants right after LAYA_DEFAULT_MAX_LEN, (b) edit the same from_env docstring lines, (c) touch adjacent import lines (#37 deletes contextlib/importlib, #9 edits the typing line two lines down), and (d) #9 inserts ~100 lines exactly where #37 deletes the function. #9 keeps without_weight_init() only as hunk context — nothing in it calls the function. Expect 3-4 small conflicts on the next main sync; resolution is mechanical (keep both constant blocks, merge both docstring sentences, keep #37's fp32 gate alongside #9's LAYA_COMPACT_BROWSER_STATE return line).
PR #9 keeps without_weight_init() only as hunk context — nothing in its diff calls it. But #37 and #9 both (a) insert constants right after LAYA_DEFAULT_MAX_LEN, (b) edit the same from_env docstring lines, (c) touch adjacent import lines (#37 deletes contextlib/importlib, #9 edits the typing line two lines down), and (d) #9 inserts ~100 lines exactly where #37 deletes the function. Expect 3-4 small conflicts; resolution is mechanical (keep both constant blocks, merge both docstring sentences, keep #37's fp32 gate alongside #9's LAYA_COMPACT_BROWSER_STATE return line).
5. Changelog behavior claims are tested [P3-none]
fp32 default, env-var precedence, non-numeric error, pre-0.3.10 laya untouched, and the version-pinned "laya unknown" message each have a test in tests/test_decision_models_laya.py.
Minor nit: [P3] int() accepts "0"/"-3"/"5_0", and laya clamps with max(1, ...), so LAYA_MPS_AMP_MIN_ROWS=0 silently means fp16 from one question — legal per docs but surprising; a range check or a doc word would help.
What I ran: gh pr checks 37 (core 3.11/3.13, windows 3.11, full: pass; browser: skipping); the prescribed pytest recipe in /tmp/pr37-review — 632 passed, 39 skipped, 96 subtests, 1 failed (tests/test_cli.py::TestStdoutIsForResults::test_outside_a_checkout..., known-broken on main, ModuleNotFoundError: openjiuwen in this venv); laya+factory tests alone: 33 passed, 2 skipped; wheel inspections of laya 0.3.9 and 0.3.20; gh pr diff 9 for the conflict analysis.
Disposition: approve. The dependency move is exactly as described, verified against the actual published wheels; evidence tooling under docs/results/laya-upgrade/ is reproducible and committed without artifacts. #9 is already in main, so sync main before merge and expect the mechanical conflicts from finding 4.
# Conflicts: # .env.example # CHANGELOG.md # docs/configuration.md # s1a/decision_models/laya.py
Laya raises any value below 1 to 1, so LAYA_MPS_AMP_MIN_ROWS=0 or a negative number passed the whole-number check and ran every MPS request in fp16. The check now asks for a whole number of at least 1, before the checkpoint loads, and the test covers 0 and -3 next to a non-number.
Why
For #28, which closed when #35 merged: #35's history carried this change and its revert. #22 wrapped
laya.loadin transformers'no_init_weightsto skip the throwaway random weight init. Laya does this itself from 0.3.9, so the wrapper and its test stubs are no longer needed.Moving the lock past 0.3.9 brings one more change. From 0.3.10 Laya answers a request of five or more questions in fp16 on MPS. In s1a only the browser front asks that many at once, on pages that offer all four operations, so one episode would mix fp32 and fp16 steps. On the 30 ticket-router tickets with six questions, fp16 moved
multilingualprobabilities by up to 0.051 and changed two of 180 decisions, both close to even (records/head-multilingual-mps-override5.jsonbelow). Keeping fp32 can cost speed: the one M5 report (NandhaKishorM/laya#109) has fp16 5 ms faster at five questions.How
LayaModel.from_envno longer wrapslaya.load, since laya skips the init itself. After loading, it raises the agent'smps_amp_min_rowsso every MPS request stays in fp32, the precision main runs today, unlessLAYA_MPS_AMP_MIN_ROWS(Laya's own variable) is set; a value that is not a whole number is a configuration error, because Laya would read it as its default of 5. A laya without the attribute (0.3.9) is left as it is.Open first:
s1a/decision_models/laya.py(from_env), thentests/test_decision_models_laya.py.What
layaextra islaya>=0.3.9.uv lock --upgrade-package layamoves the lock from 0.3.5 to 0.3.20, the release system1-omni's Laya worker runs; only the laya entry changes. 0.3.26 is the latest today; say if you would rather lock that.without_weight_init()and itstransformers.initializationtest stubs are gone.--model layakeeps MPS requests in fp32 unlessLAYA_MPS_AMP_MIN_ROWSis set. Nothing changes on CPU or CUDA. Callers change nothing; settingLAYA_MPS_AMP_MIN_ROWS=5restores Laya's default.system_onetest now pins the version, so it passes with the laya extra installed too).docs/configuration.mdand.env.exampledescribe the variable.docs/results/laya-upgrade/: the comparison below, as a README with the steps andrun.sh. The recorded outputs are attached to this PR rather than committed, as the artifact hygiene guide asks.Verification
Run by me on
b6edc84, M1 Pro, Python 3.14, with the laya extra (0.3.20) unless noted.uv run ruff format --check . && uv run ruff check . && uv run ty check: all pass.uv run pytest -qandscripts/smoke.sh: 607 passed, 43 skipped;smoke: ok(core install). In a fresh environment whose packages are not byte-compiled yet, Python 3.14 printsSyntaxWarnings from pysbd and openjiuwen on stderr, which fails smoke's one-line check fordecideandtest_s1a_home_from_the_dotenv_file_places_the_logs_and_the_runtime_root. Main fails the same way there;uv sync --compile-bytecodeavoids it on both.CHANGELOG.mdand the docs say what the code does now.CI (core install, no laya extra) passes on
b6edc84.Demo / evidence
Task.
--model laya's backend,LayaModel.from_env(), answers the 30 tickets ins1a/agents/_data/ticket_router_eval.jsonl: one choice, one noul and one six-question request each. Success is every choice, probability, confidence and noul value equal between main and this branch.Actions.
docs/results/laya-upgrade/README.mdhas the steps andrun.shruns them: it checks out main (3008e8f, laya 0.3.5) and this branch's last laya commit (d0b3d29, laya 0.3.20), installs each lock, and runs both checkpoints on CPU and MPS, three fresh processes per side, alternating. Two controls: this branch withLAYA_MPS_AMP_MIN_ROWS=5on MPS, and main without its weight-init skip on CPU.Result. An actual run by me, 2026-10-05, M1 Pro, macOS 26.1, checkpoint revision
7b928d8, at222e656andc197fe6: main and this branch before the rebase onto main after #35 merged. Locked package versions and the in-process path are the same at the commits above (the README lists the differences); the 28 outputs arerecords/inpr37-evidence.zip, attached to this PR, each naming its commit, versions, device and Laya variables.python compare.py records/printed:identicalbetween main and this branch, 3 + 3 runs each;LAYA_MPS_AMP_MIN_ROWS=5:multilingualchangedt_6cb98186dde0(pick, logistics to returns) andt_f5251390fb88(cancel, 0.4959 to 0.5074), largest probability change 0.051;english0.010, no decision changed;english) and 42.5 s (multilingual), against 6 to 10 s on either side with it.Limits. Load and latency columns compare the two sides of a row only: the machine had other work running (1-min load about 6 to 10), and the first
englishCPU load of each side read the model files from disk. Not covered: thetyped-decisionscheckpoint, a local checkpoint path, transformers 4.x, CUDA, and Apple chips other than the M1 Pro.Video. The comment with the clip runs the same three tickets in process and through system1-omni (main
4a67957, worker in fp32, not compiled, behindomni-jev): same routes and probabilities. Its traces areclip/in the same zip.