docs(tuning): add an interactive auto-tune explainer - #288
Conversation
🔴 PR size: 2180 lines — over the 800-line hard limit
Target is 400 lines excluding tests, hard stop around 800 — see |
515f368 to
95950b8
Compare
Write section 1. The load-bearing claim is that HZ_PER_STEP and HL_HZ_PER_STEP are not what production uses -- they feed only the derived ESTIMATED_MICROSTEPS_PER_HZ constants, which in turn are imported only by the simulation IOC to seed SCALE. Live tuning reads the measured per-cavity SCALE PV instead. Corrects three citations from the plan against current main: - the estimates are consumed in tuner_service.py's SCALE startup handler (104-111), not at the pvproperty declaration (98) - the HL step inversion spans stepper.py:355-356 - adds frequency_tuning.py:479 and :360 for the signed measurement and the SCALE_CALC.B write, which the plan asserted without citing
Write section 3. Uses Variant A: find_chirp_range still guards on the raw signed value on main, so the negative in-loop entry point bypasses the 400 kHz cap. PR slaclab#286 is the fix in flight; when it merges this subsection should be replaced with Variant B from the plan. Verified before writing: - use_sela=True has exactly one caller (setup_cavity.py:219), so the 'exactly one caller' claim stands - setup_tuning passes a positive default 50000; check_detune passes chirp_freq_start * 1.1, which is negative Corrects the plan's caveat: the abs() normalization lives in set_chirp_range (cavity.py:669-677), not in find_chirp_range. The net behaviour the plan describes is right, but the attribution was not. Also cites cavity.py:1155 for the guard itself.
Write section 4. All five citations verified exact against main: linac_utils.py:154-157 (the four TUNE_CONFIG values), cavity.py:747 (RESONANCE on move_to_resonance success), cavity.py:846 (OTHER on _auto_tune entry), cavity.py:164 (DF_COLD), stepper.py:55 (NSTEPS_COLD). The diagram's colours are the literal :root token values rather than var() references, since SVG presentation attributes resolve custom properties less reliably than CSS and this page is opened offline on control-room browsers. Added a comment so a future theme change knows to mirror them here. Not verified: how the diagram actually looks. There is no dev server for this static page, so this was checked by parsing only -- markup is well-formed, the marker id resolves, and the geometry fits the viewBox without overlap. Someone should still open it.
Write section 5 and apply the Task 8 Step 0 renumber: insert the commissioning section and bump 'How it fails' from 5 to 6. The page now has all six sections. Every citation in Step 2's check matched main exactly -- no corrections needed. Also confirmed the caveat's two omissions are still real: check_state_for_stage has 0 occurrences and the UI controller file does not exist on main, so the deferred-to-a-later-PR framing is honest. The renumber broke two references the plan did not anticipate. Task 4's simulator JS pointed readers at 'section 5' for the fault explanations, which is now the commissioning section -- updated both the operator-facing fault-picker note and the setFault comment to say section 6.
Write section 6: eight failure modes, six of them injectable into the section 2 simulator. All eight Python citations verified exact against main, including the ~10 s abort worst case (a 5 s settle sleep at stepper.py:385 plus the 5 s poll interval at :404). Buttons are wired through Task 4's existing setFault(), which already owns the fault state and syncs the section 2 picker -- no second copy of that state, as the plan requires. mountFailureInjectors() runs after mountSimulator() so faultSelect exists when setFault syncs it. Not verified: Step 3's click-through. There is no browser or JS runtime available here, so the buttons were checked structurally only -- all six data-fault values cross-check against FAULT_ORACLE with no dead buttons either way, the #the-loop scroll target exists, and the script block parses balanced. The per-fault verdicts themselves are already asserted by the page's own selfCheck(), which runs runAutoTune over FAULT_ORACLE and checks outcome, move count, fire iteration and log integrity -- so opening the page and seeing SELF-CHECK PASSED covers the behaviour. What still needs a human: that clicking each button scrolls to section 2 and that Reset clears the fault. One thing to watch on that pass: selfCheck uses FAULT_BASE, whose SCALE is 1.4/256 ~ 0.0054688, while the slider default is the nearest grid point 0.0055. That 0.6 % gap should not change any verdict -- slip overshoots its budget roughly tenfold -- but slip is the one to confirm.
Task 10. Audit results, all against real Python: - stepper_tol_factor(10e6) computes to exactly 1.090000, matching TOL_ORACLE's [10000000, 1.09]. The 10e6 -> 1.05 breakpoint in step_tol_des is dead -- `ranges` jumps (5e6, 50e6) and never uses it -- and the page's QUIRK note already says so. - Ported STEP_TOL_DES and TOL_RANGES match linac_utils.py:243-250 and :251 exactly. - Tolerances 50/500 Hz, STEPPER_TEMP_LIMIT 70 C, SAFE_PULSED_DRIVE_LEVEL 10, PIEZO_CENTER_VOLTAGE 25: all confirmed. - No text implies HZ_PER_STEP is used in production; the one remaining mention is explicitly labelled 'nominal' for a worked example, and its linac_utils.py:145-147 span correctly covers MICROSTEPS_PER_STEP through HZ_PER_STEP. Offline check clean (no http, img, src, @import, integrity, fonts), no innerHTML, 17/17 drift tests pass. Step 5, the browser pass, is NOT done -- no browser or JS runtime is available in this environment. Outstanding for a human: console reads SELF-CHECK PASSED with no failure banner, sliders update their readouts, the six inject buttons scroll and fire, and the page reads without horizontal scroll at 1280 px.
GitHub never renders HTML from a repo -- the blob view shows source and the raw URL is served text/plain with nosniff. This repo has no GitHub Pages and no site generator, so the index link resolves to 2102 lines of markup for anyone reading docs/ on github.com, which is where it is most likely to be read. Says so in the table rather than leaving the reader to click and wonder. Standing up Pages for docs/ would fix it properly, but that is infrastructure beyond this PR.
…onstrates Found by running the page. The button sat on the 'Operator abort, stepper / StepperAbortError' row, but the simulator's 'abort' fault models the cavity path -- check_abort() calls turn_off() and raises CavityAbortError (cavity.py:1054-1061). Clicking Inject on the stepper row therefore produced 'CavityAbortError -- aborted, and RF is now off', demonstrating the row below it. Moved the button to the cavity row, which is what it injects. The stepper row keeps no button: the simulator has no separate StepperAbortError fault.
…ab#286 and slaclab#287 main moved four commits and invalidated three passages plus half the citations. slaclab#286 (ed8c13e) fixed the chirp-range cap. find_chirp_range now normalizes with abs(int(...)) at cavity.py:1196, so the negative in-loop entry point no longer bypasses the 400 kHz cap. Section 3 shipped Variant A, which documented that bypass as live behaviour -- replaced with Variant B. slaclab#287 (d5b1b2d) fixed the zero-step hang. The loop-oracle case asserted 'in the real Python this spins indefinitely'; cavity.py:884-900 now raises DetuneError naming the suspect SCALE. Ported the guard into runAutoTune at the same position (right after est_steps, before the move), added a zero-step outcome and verdict, and changed the oracle case from no-converge to zero-step/0 moves. Also retitled the no-converge label, which pointed at the truncation case that no longer produces it. slaclab#270 (ca62991) landed the tuning UI. Section 5's caveat said the re-run gates and the controller were 'not on main' and deferred them; both exist now, so it names them and says the loop page is scoped away from them deliberately rather than pending. Citations: 18 of 37 cite-span references were stale, plus two more in JS comments that a cite-span-only audit misses. slaclab#287 added ~45 lines inside _auto_tune and slaclab#270 reshaped frequency_tuning.py heavily. Re-resolved every one by anchor text; a sweep of all 50 file:line references in the document now reports zero landing on blank or closing-paren lines.
ada51c3 to
577c87c
Compare
…ivable Three additions on top of the resync in 577c87c. Widened the starting-detune slider to -50 kHz..200 kHz. A cavity coming off cooldown can sit hundreds of kHz out, so +/-50 kHz understated the envelope tuning actually walks in from. This sharpens section 2 rather than diluting it: the tolerance factor tightens from 1.369x at 5 kHz to 1.0369x at 200 kHz, so the calibration-error window visibly narrows as the detune grows. 400 kHz is the most that would ever be worth offering, since that is where find_chirp_range's cap stops widening -- past it the detune cannot be measured at all. Added a bad_scale injection so slaclab#287's guard can be watched rather than only read about. The sliders cannot reach the zero-step state -- it needs a detune below 0.11 Hz while also above the 5 Hz tolerance floor -- and widening the scale slider far enough would put values no cavity has in front of the reader. The injection rewrites SCALE mid-tune instead, which is also the honest route in: microsteps_per_hz is re-read every iteration, so a bad _apply_hz_per_step probe takes effect on the next move. Needed a mutable effective scale, kept separate from microstepsPerHz because expected_steps and the step budget are computed once before the first move, so a mid-tune SCALE change moves the estimate without moving the budget. Also gave it a failure-table row, and noted on the step-budget row that the runaway message now reports an unchanged detune -- the stuck-tuner signal slaclab#287 added. selfCheck() now asserts every Inject button sits on a row whose Raises column names the exception the simulator reports. That is the check that would have caught the abort button being on the StepperAbortError row while injecting the cavity path; verified it fails if that arrangement is restored. SELF-CHECK PASSED (14 tol-factor, 12 loop, 7 fault cases). Generated with AI Co-Authored-By: SLAC AI
There was a problem hiding this comment.
Pull request overview
Adds a new interactive, offline HTML explainer documenting the _auto_tune() convergence loop (shared by tuning GUI, auto setup, and commissioning), and introduces a drift test to keep the explainer’s hand-copied constants synchronized with the real Python implementation.
Changes:
- Add
docs/explainers/auto_tune.html: a self-contained interactive simulator + narrative explainer for_auto_tune(). - Add
tests/docs/test_auto_tune_explainer.pyto re-derive the HTML oracle values fromstepper_tol_factor()on every pytest run. - Include
docs/*.htmlin the sdist and link the explainer fromdocs/index.md.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
docs/explainers/auto_tune.html |
New interactive explainer + JS port/oracles + in-page self-check. |
tests/docs/test_auto_tune_explainer.py |
Drift test that parses TOL_ORACLE from the HTML and validates it against Python. |
tests/docs/__init__.py |
Marks tests/docs as a package (empty). |
docs/index.md |
Adds a docs index entry pointing to the explainer. |
MANIFEST.in |
Includes docs/*.html in the source distribution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The docs are currently only readable as files. This builds them into a site and deploys it on push to main. MkDocs specifically, because every internal doc link is a relative .md path and README.md links into docs/index.md three times. MkDocs rewrites .md links to built URLs, so they keep working both on the site and in GitHub's blob view. Jekyll-from-/docs would 404 on all of them unless each were rewritten to .html, which would break GitHub browsing. A bare artifact upload would serve .md as plain text. strict: true is deliberate -- these docs cross-reference each other heavily, so a silent 404 is how the site rots. It paid for itself immediately: it caught getting_started.md linking ../README.md, which is outside docs_dir. That is now an absolute GitHub URL, so it resolves both on the site and on github.com. check-manifest runs in CI against the ignore list in pyproject.toml, and a new root mkdocs.yml would have failed package/verify. Added there and to MANIFEST.in alongside the existing Makefile exclusion. Actions pinned to full SHAs per the org policy in .github/dependabot.yml. Deliberately contains no reference to the auto-tune explainer in slaclab#288, so this can land independently. MkDocs copies non-markdown files from docs_dir into the site regardless of nav, so when slaclab#288 lands its index link resolves on the site with no change needed here -- verified by building with the explainer present and absent from nav.
The docs are currently only readable as files. This builds them into a site and deploys it on push to main. MkDocs specifically, because every internal doc link is a relative .md path and README.md links into docs/index.md three times. MkDocs rewrites .md links to built URLs, so they keep working both on the site and in GitHub's blob view. Jekyll-from-/docs would 404 on all of them unless each were rewritten to .html, which would break GitHub browsing. A bare artifact upload would serve .md as plain text. strict: true is deliberate -- these docs cross-reference each other heavily, so a silent 404 is how the site rots. It paid for itself immediately: it caught getting_started.md linking ../README.md, which is outside docs_dir. That is now an absolute GitHub URL, so it resolves both on the site and on github.com. check-manifest runs in CI against the ignore list in pyproject.toml, and a new root mkdocs.yml would have failed package/verify. Added there and to MANIFEST.in alongside the existing Makefile exclusion. Actions pinned to full SHAs per the org policy in .github/dependabot.yml. Deliberately contains no reference to the auto-tune explainer in #288, so this can land independently. MkDocs copies non-markdown files from docs_dir into the site regardless of nav, so when #288 lands its index link resolves on the site with no change needed here -- verified by building with the explainer present and absent from nav.
Review from Sebastian: - The piezo does not reject microphonics. It runs a slow (~few Hz) frequency feedback that compensates drift. Applied his wording in the role row, and followed it through to the two other places the page implied active microphonics rejection: the speed row's "closes a feedback loop", which now reads as a contradiction next to "slow feedback", and the piezo-centring paragraph in section 3. - NSTEPS_COLD is not a stepper position. It is the signed return-trip step count, resonance back to cold landing (frequency_tuning.py:967-971) — the cold landing frequency is recorded before the stepper moves at all. Also Copilot: read the explainer with an explicit UTF-8 encoding. The page contains Δ, ° and ×, and the repo's only other read_text already pins it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e failure When the direction probe moves 50,000 steps and sees less than 100 Hz of detune change, the failure message told the operator to check "that the stepper is mechanically connected and the cavity is at 2 K". The 2 K half cannot be the cause. Reaching that branch means both detune readings succeeded, and detune is only visible when the cavity is at 2 K — stepper.py:349 says exactly that about check_detune. A warm cavity does not produce a small probe delta; it produces no usable detune at all. So the hint sends the operator to look at a cryo plant that is demonstrably fine, past the one cause that fits: the stepper turning without moving the tuner. Raised by Sebastian in review of slaclab#288. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What this changes
Adds an interactive, offline HTML explainer for
Cavity._auto_tune()— the single convergence loop behind the tuning GUI, auto setup, and RF commissioning — plus a drift test that re-derives the page's numbers from the real Python on everypytestrun.Six sections: what tuning moves, the loop (with a drivable simulator), where the detune number comes from, tune states, the commissioning stages, and how it fails. The failure table's 7 Inject buttons drive the section 2 simulator so you can watch each fault play out.
Operator-visible
None. Documentation only — no PV writes, no application behavior changes, no launch changes. Docs HTML now ships in the sdist (
MANIFEST.in), which does not affect the wheel or anything operators run.Scope
Docs-only; no
src/changes. Purely additive — 5 files, no deletions.Two supporting files ride along:
MANIFEST.ingains one line (sdist only; the wheel is untouched).tests/docs/test_auto_tune_explainer.pyasserts the page's constants against the real Python.Over the 400-line target, but the page is a single document — splitting it would ship half an explanation. The bulk is one 2172-line self-contained HTML file with no runtime dependency on anything else in the repo. The working plan and design spec used to build it were dropped from this branch rather than shipped; they were process artifacts, not deliverables.
The
find_chirp_range±400 kHz cap bypass found while writing this was fixed separately in #286, which has since merged; §3 documents the fixed behaviour.Decisions worth recording
Interactive rather than markdown. The loop's behaviour is hard to convey statically: whether it converges depends on the interaction of undershoot, calibration error and the tolerance budget. Section 2 lets the reader move those and watch the outcome flip, which prose cannot do.
Resynced against
mainafter #270, #286 and #287 merged mid-review. Three passages went stale and were rewritten, not patched over:abs(int(...))atcavity.py:1196). §3 had documented that bypass as live behaviour; it now documents the fix.cavity.py:884-900now raises. The JS port carries the same guard at the same position, with a newzero-stepoutcome, and the oracle case was changed accordingly.main" and deferred them. Both exist, so §5 names them and scopes them out deliberately instead.The zero-step guard is drivable, not just described. #287's fix needed a demonstrable path, and the sliders cannot reach that state — it needs a detune below 0.11 Hz while also above the 5 Hz tolerance floor, and widening the scale slider far enough would put values no cavity has in front of the reader. So it is a
bad_scaleinjection that rewritesSCALEmid-tune. That is also the honest route in:microsteps_per_hzis re-read every iteration, so a bad_apply_hz_per_stepprobe takes effect on the next move. It needed a mutable effective scale, kept separate frommicrostepsPerHzbecauseexpected_stepsand the step budget are computed once before the first move — so a mid-tuneSCALEchange moves the estimate without moving the budget.Starting-detune slider widened to −50 kHz…200 kHz (review feedback). A cavity off cooldown can sit hundreds of kHz out, so ±50 kHz understated the envelope tuning actually walks in from. This sharpens §2 rather than diluting it: the tolerance factor tightens from 1.369× at 5 kHz to 1.0369× at 200 kHz, so the calibration-error window visibly narrows as the detune grows. 400 kHz is the most worth offering at all, since that is where
find_chirp_range's cap stops widening — past it the detune cannot be measured.selfCheck()now binds each Inject button to its table row. Theabortbutton sat on theStepperAbortErrorrow while injecting the cavity path, which raisesCavityAbortErrorand turns RF off — the exact distinction §6 exists to draw, so a reader clicking that row was shown the other path's outcome. Fixed, and the check asserts every button's row names the exception the simulator reports. Verified it fails if the old arrangement is restored.The drift test lives in
tests/, not as an uncommitted audit script. The page restates Python constants in JavaScript so it works offline. Its in-pageselfCheck()only proves the page agrees with itself. A page whose entire value is trustworthy numbers needs CI to catch the day a constant moves — so the test parsesTOL_ORACLEout of the HTML and re-derives every row from the real importedstepper_tol_factor.Deferred scope
Section 5 documents the commissioning backend only. The per-stage re-run gates (
_check_state_for_stage_2/3/4) and the 1,700-linefrequency_tuning_controller.pylanded with #270, but they are out of scope for a page about the convergence loop. §5 names them and says so, rather than leaving the reader to notice the gap.Learning reviewer
@RDPorter — this page was written for you. Worth reading §1 (why
HZ_PER_STEPis not what production uses) and §3 (the chirp-range cap bypass). No approval needed.To open it: GitHub will not render it — HTML in a repo is served as source, and there is no Pages site here. Either check the branch out:
gh pr checkout 288 --repo slaclab/sc_linac_physics && open docs/explainers/auto_tune.htmlor just grab the one file:
The URL tracks the branch rather than a pinned commit — an earlier pinned SHA was orphaned by a rebase and quietly served a stale page. Either works offline; the page has no external references at all. The docs index now says this too, so the next reader does not have to find out by clicking.
Testing
pytestpasses — 3154 passed, 3 skipped (rebased onto 9.34.0)blackandflake8both clean.Browser pass: done. Served locally with
python3 -m http.serverto run the real JavaScript, since the page is a static file with no dev server.SELF-CHECK PASSED (14 tol-factor cases, 12 loop cases, 7 fault cases), no failure banner above the<h1>.selfCheck()).All
data-faultvalues cross-check againstFAULT_ORACLEwith no dead buttons either way; no external references, noinnerHTML.