Decouple tests from measured benchmark parameters - #70
Conversation
Refreshing benchmarks/assembly_params.json scaled every energy value by ~2.7x and broke three tests that asserted absolute magnitudes. Tests now own their energy configs and assert scale-invariant relations instead.
jcc_symbolic, zext_register and the two adjcallstack pseudo-ops were absent from tests/assembly_params.json, so the MIR estimator costed them at defaultEnergy_ (1.0) with only a warning. The fixture now fails when the pass reports any missing key.
E_safe is capacity - E_pro - E_epi - (N_reg - 2) * reg_restore_energy, which tests/rockclimb_params.json makes 471.87, not ~476.
max_unroll=4 matched RockClimbMaxUnrollFactorOpt's default, so the flag test passed whether or not the flag was read; it now uses 3. UnrollLoop only reports a full unroll when Count equals the trip count, making the N == K check on that log line vacuous.
E_loop is logged before refineLoopBudgetWithConvergence and numIt after, so reading availableEnergy alongside E_loop compared two different budgets. The assertion built on it was implied by the E_loop one and is gone.
Comparing discovery against _DEFAULT_CAPS alone still passes if 50uF is dropped from both.
|
Review addressed in 5 commits on top of f3eed6b — missing energy keys (HIGH). Confirmed. 8dabac0 — wrong E_safe in docstring (MEDIUM). Confirmed, and it was my error: ee25e46 — assertions that cannot fail (LOW x2). Both confirmed. 3184da3 — pre/post-convergence mixing (MEDIUM). Confirmed at 085741d — lost 50uF guard (MEDIUM). Restored an explicit Not done: I did not restore the Correction to this PR's description: the "within 0.69–1.33x" claim has three exceptions — Also extended 252 passed; ruff and pyright clean. |
crc.c and aes.c are copied verbatim; both produce byte-identical pass output from the new location.
1 participant
Commit 93fded7 refreshed
benchmarks/assembly_params.jsonfrom new board data, scaling every energy value by ~2.7x. Three tests failed:test_rockclimb.py::TestCallHandling::test_external_call_costed_as_expensive_instructiontest_rockclimb_preprocess.py::test_compile_rockclimb_handles_full_unroll_in_nested_loopstest_schematic.py::test_schematic_o3_dijkstra_loop_budget_uses_rare_inner_branchNot a regression — the commit is data-only, and reverting just that file makes all three pass. Each assertion was pinned to an absolute magnitude read out of a file that gets regenerated whenever the board is re-characterized.
Changes
New
tests/assembly_params.json— 153 synthetic instruction costs from an explicit tier model (1.5 x units; an immediate source adds 1, a memory source adds 2, a memory destination adds 1 more when read back;__mspabi_diviand friends are set far above any test capacity). Every value lands within 0.69–1.33x of what the tests were tuned against, so no threshold moved. The file documents that it must never be regenerated frombenchmarks/.Other decouplings
test_rockclimb_preprocess.pyreadbenchmarks/config_5uF.json/config_1uF.json, which carry measuredE_pro/E_epi/ register costs (14 and 6 commits of churn). Replaced with a_write_board_like_confighelper using round values in the same regime. Also swappedbenchmarks/sample_energy_config_ir.jsonfor the existingtests/estimator_ir_weighted.json, with capacities written as multiples of a namedCONSTANT_LOOP_ITER_ENERGYrather than bare20.0/200.0.fully unrolled ... N=4 K=4now asserts that some loop fully unrolled withN == K. The regression being guarded is a crash on full unroll inside a nested loop;N=4was incidental toaes.c.inner > 70,outer > 2400,numIt < 25withouter.e_loop >= inner.e_loop * inner.max_trip_countandouter.num_it <= available_energy / that. This states the intent directly (the outer budget folds in the rarely-taken inner branch) and holds under any scaling.test_capacitor_support.pyhardcoded the live capacitor list and243000.0; it now compares against_DEFAULT_CAPSandCAPACITY_MAP.Verification
HEAD~1'sassembly_params.jsonand both old capacitor configs restored — the coupling is gone in both directions.benchmarks/assembly_params.jsonby 0.25x and 4x.Not addressed
CAPACITY_MAPinscripts/ckpt/analysis/strip_mining.pydisagrees with the configs: 50uF = 243000 and 1uF = 4860, whileconfig_50uF.jsonsays 182000 andconfig_1uF.jsonsays 3640. One is stale. That is a production bug, not a test one, so it is left alone here.test_rockclimb_preprocess.pystill compiles the realcrc.c/aes.c, and the dijkstra test still names the IR loop headersfor.body18.i/while.body.i. Those are the actual regression subjects; freezing copies would duplicate hundreds of lines, and if the sources change these fail loudly with a missing-log-line message rather than a confusing threshold miss.