Skip to content

fix: radar_classification's run() dispatched to a dead debug function, not main() - #34

Open
musicalplatypus wants to merge 5 commits into
TexasInstruments:mainfrom
musicalplatypus:pr/radar-main-live-entrypoint
Open

fix: radar_classification's run() dispatched to a dead debug function, not main()#34
musicalplatypus wants to merge 5 commits into
TexasInstruments:mainfrom
musicalplatypus:pr/radar-main-live-entrypoint

Conversation

@musicalplatypus

@musicalplatypus musicalplatypus commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

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's run() called main_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 of main(), which is fully wired to this codebase's shared training infrastructure.

Concretely, main_debug():

  • Never calls quantization_wrapped_model — the "QuantTrain" pass produces a second, mislabeled float-training run instead of an actual quantized model.
  • Never calls resume_from_checkpoint — checkpoint resume is silently a no-op.
  • Hardcodes a fixed random seed and a plain SGD optimizer with no LR scheduler, unlike 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 0 and --quantization 1 — the quantized pass now correctly shows QuantTrain phase 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-model either (neither main() nor main_debug() called compile_model_if_enabled) — that's added in #35, on top of this fix making main() reachable at all.

Changes

  • run() now dispatches to main, matching every other reference script's pattern
  • New regression test pinning the dispatch target so this can't silently regress back to main_debug
  • --compile-model support 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 in main(), which was previously unreachable)

CI

CI is red on all three OS jobs for this PR — but it's also red on main at this PR's base commit (6f2c11cc), from test_config_validation.py's radar_point_cloud_classification example 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

  • New regression test: tests/test_radar_entrypoint_uses_main.py
  • Manual E2E on synthetic fixture, both quantization=0 and quantization=1, both completing cleanly with the expected artifacts
  • Full existing test suite passes locally (2 pre-existing, unrelated CI failures — see CI section above)

t5fkg8d44d-beep and others added 3 commits August 13, 2026 12:43
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants