Skip to content

test: pin resolve_runtime_dtype and compute-bound ceiling fallbacks (PRPUNDIT-11, PRPUNDIT-15) - #1330

Open
jiagaoxiang wants to merge 5 commits into
AMD-AGI:mainfrom
jiagaoxiang:testgap/PRPUNDIT-15
Open

test: pin resolve_runtime_dtype and compute-bound ceiling fallbacks (PRPUNDIT-11, PRPUNDIT-15)#1330
jiagaoxiang wants to merge 5 commits into
AMD-AGI:mainfrom
jiagaoxiang:testgap/PRPUNDIT-15

Conversation

@jiagaoxiang

@jiagaoxiang jiagaoxiang commented Aug 28, 2026

Copy link
Copy Markdown

Summary

  • Covers resolve_runtime_dtype provenance branches and that workload precision does not drive weight dtype.
  • Directly asserts compute_compute_bound_ceiling_tok_per_sec weight_bytes fallback stays nonzero, and missing peak / dtype / weight degrade to 0.0.

Covers test gaps PRPUNDIT-11 and PRPUNDIT-15 (merged from #1326 into this PR).

Test plan

  • PYTHONPATH=src pytest src/hyperloom/inference_optimizer/tests/test_roofline_ceiling_perfmodel_units.py::test_resolve_runtime_dtype_priority_and_ignores_workload_precision src/hyperloom/inference_optimizer/tests/test_roofline_ceiling_perfmodel_units.py::test_compute_compute_bound_ceiling_fallback_and_degrade_to_zero

Cover quantization, pre-quantized meta, --dtype, bf16-floored fallback, and that workload.precision does not drive weight dtype.
…IT-15)

Directly assert the weight_bytes fallback stays nonzero and missing peak/dtype/weight degrade to 0.0.
@jiagaoxiang
jiagaoxiang requested a review from a team as a code owner August 28, 2026 07:13
@jiagaoxiang

Copy link
Copy Markdown
Author

Merged #1326 into this branch and resolved the test_roofline_ceiling_perfmodel_units.py conflict. This PR now has both resolve_runtime_dtype and compute_compute_bound_ceiling_tok_per_sec tests. Safe to merge after or instead of #1326.

@jiagaoxiang jiagaoxiang changed the title test: pin compute_compute_bound_ceiling_tok_per_sec fallbacks (PRPUNDIT-15) test: pin resolve_runtime_dtype and compute-bound ceiling fallbacks (PRPUNDIT-11, PRPUNDIT-15) Aug 28, 2026

@zoroyihan7 zoroyihan7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the four dtype branches and the three degrade-to-0.0 exits are all reached, and asserting the ceiling == 0.0 directly (rather than via bound_kind == "memory", the way test_conc_sweep_ceiling.py::test_dense_fallback_no_moe_fields does) is exactly right and avoids that masking pattern.

I mutation-tested the production code against these tests, and three of the behaviors the ticket asked for aren't actually pinned — the tests pass, but they'd also pass if the code regressed.

1. achievable-over-vendor precedence isn't pinned. roofline_ceiling.py:1012 is:

peak_tflops = _resolve_achievable_tflops(...) or _resolve_peak_tflops(...)

The test patches achievable → 100.0 and vendor → 0.0, so achievable or vendor and vendor or achievable both yield 100.0. Swapping the operands leaves 44/44 green. Patch vendor to a different positive value (e.g. 500.0) and assert the result derives from 100.0.

2. The vendor-dense fallback leg is never exercised. The only case with achievable == 0 also has vendor at 0.0, so it only reaches the degrade exit. A case with achievable == 0.0, vendor > 0 asserting a positive ceiling would cover it.

3. Dtype priority is pinned for 2 of 4 adjacent pairs. Branch 1-over-3 (the --quantization fp8 --dtype bfloat16 case) and 3-over-4 are pinned, but 1-vs-2 and 2-vs-3 have no case where both sources are present — hoisting quantization_config above the --quantization branch, or --dtype above quantization_config, both leave 44/44 green. Two cases close it: recognized --quantization fp8 with a pre-quantized meta (weight_dtype_bytes=0.5) asserting source == "server_args_quantization"; and a pre-quantized meta plus --dtype float32 asserting source == "quantization_config".

Three smaller escapes found by mutation, worth a line each if you're already in here:

  • Widening if 0 < meta_w_bytes < 2.0: to if 0 <= meta_w_bytes <= 2.0: stays green — neither the 2.0 upper edge nor the meta == 0 (unknown dtype) lower edge is pinned.
  • Deleting the documented bf16 activation floor (act_bytes = max(..., 2.0)) stays green — every case uses bfloat16/float32/absent, none below 2 bytes.
  • compute_precision_tag is never asserted in any branch; replacing _compute_tag_for_bytes(wb) with a constant stays green, even though that tag is the key for the downstream TFLOPS lookup.

Nits, take or leave: gpu_type="unknown-gpu" in the degrade case is inert since both resolvers are patched for the whole body (the name implies a real table miss that isn't happening); and both tests are monolithic multi-scenario functions, so an early failure hides later scenarios — parametrize would localize them.

Happy to re-review once the precedence cases are in.

@zoroyihan7 zoroyihan7 added the skip-e2e-test It's a PR that doesn't need to be e2e tested label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-e2e-test It's a PR that doesn't need to be e2e tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants