fix: radar_classification's run() dispatched to a dead debug function, not main() - #34
Open
musicalplatypus wants to merge 5 commits into
Open
Conversation
Post-fix (main() live per Task 1): CPU 0.621s/epoch, MPS 1.066s/epoch (1.72x slower) -- gap did not close vs pre-fix's 1.90x. Root cause: compile_model_if_enabled/apply_hardware_defaults are never called anywhere in radar_classification/train.py (only wired into the timeseries_* reference scripts), so torch.compile/AMP were off for all four runs regardless of which function run() dispatches to. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Whole-plan review flagged that the Goal/Architecture sections still asserted the disproven premise (main() wired to compile_model_if_enabled) 130+ lines above the actual Results. Ledger and Results were already honest; this fixes the presentation for a top-down reader.
Claimed main_debug() alone lacked compile_model_if_enabled/apply_hardware_defaults, implying main() already had it -- at this commit neither function does (that's added in a later change). Corrected to describe what's actually true at this point: main() is the one that CAN be extended with it, not that it already is.
Stripped the 'For agentic workers: REQUIRED SUB-SKILL...' header and normalized checkboxes from - [ ] to - [x] -- every task described is done.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is part of a 4-PR stack: #34 (this PR) → #35 → #36 → #37. Recommended merge order is numeric; each later PR's diff currently includes the earlier ones' commits until they merge, since none have landed yet.
Summary
radar_classification/train.py'srun()calledmain_debug()— a leftover notebook-parity harness (its own comment: "Following as close as possible steps from jupyter notebook to test if model learning plateau is coming from training loop") — instead ofmain(), which is fully wired to this codebase's shared training infrastructure.Concretely,
main_debug():quantization_wrapped_model— the "QuantTrain" pass produces a second, mislabeled float-training run instead of an actual quantized model.resume_from_checkpoint— checkpoint resume is silently a no-op.main()'s configurable setup shared with every other reference script.This was a genuine regression risk:
main()had zero execution history for radar before this fix. Verified end-to-end on a synthetic fixture with both--quantization 0and--quantization 1— the quantized pass now correctly showsQuantTrainphase log entries and exports a second (QDQ) model, which it silently never did before.Note: at this PR's state,
main()does not yet support--compile-modeleither (neithermain()normain_debug()calledcompile_model_if_enabled) — that's added in #35, on top of this fix makingmain()reachable at all.Changes
run()now dispatches tomain, matching every other reference script's patternmain_debug--compile-modelsupport for radar is added in the follow-up PR (feat: wire compile_model_if_enabled into radar, image, and audio classification #35), which this PR's fix is a prerequisite for (compile wiring only exists inmain(), which was previously unreachable)CI
CI is red on all three OS jobs for this PR — but it's also red on
mainat this PR's base commit (6f2c11cc), fromtest_config_validation.py'sradar_point_cloud_classificationexample config (added by PR #13) not being recognized by the task-type/model registries. This PR doesn't touch that config or those registries and doesn't change the failure set.Test plan
tests/test_radar_entrypoint_uses_main.py