From c73501a8c302751242354a04d7b74049e3875ac6 Mon Sep 17 00:00:00 2001 From: Ruslan Rakhimov Date: Mon, 24 Aug 2026 20:29:01 +0300 Subject: [PATCH 1/2] fix(bench): skip an unsupported slot count instead of aborting the sweep The default sweep derives slot counts as [experts, int(0.4 * layers * experts)] per profile. For minimax-m2.5-marlin that second value is 6348, which OffloadMoeCache rejects because the marlin backend caps padded experts at 1024. The ValueError was uncaught, so the run died on the 4th of 8 profiles and the remaining four never ran -- and the table it left behind looked finished rather than truncated. Catch it in print_table and skip that one combination with a printed line. Clamping was the alternative and would have been worse: it changes the size the row measures without saying so. Build the cache before printing the header. The header and column titles used to be printed first, so a skip there would leave an orphan header with no rows under it -- the same shape the abort produced. The limit stays the backend's: the benchmark asks by constructing rather than copying 992 into a file that would not learn when a backend changes. Fixes #146 --- benchmarks/bench_offload_cache_copy.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/benchmarks/bench_offload_cache_copy.py b/benchmarks/bench_offload_cache_copy.py index a2434a66..2fc99849 100644 --- a/benchmarks/bench_offload_cache_copy.py +++ b/benchmarks/bench_offload_cache_copy.py @@ -198,6 +198,16 @@ def print_table( ) -> None: per_expert = expert_bytes(profile) cache_gib = cache_slots * per_expert / 2**30 + # Build before printing: a backend that refuses this slot count would otherwise leave a + # header with no rows under it, which is what the abort already looked like. The limit is + # the backend's (marlin caps padded experts at 1024) -- ask it by constructing rather than + # copying the number here, where it would drift the first time a backend changes. + try: + cache = make_cache(profile, cache_slots, device) + except ValueError as exc: + print(f"\n{name} @ cache_slots={cache_slots}: SKIPPED -- {exc}") + return + print( f"\n{name} ({profile.quant_format}, {len(bank_specs(profile))} banks, " f"L={profile.layers} E={profile.experts} k={profile.topk}) " @@ -207,7 +217,6 @@ def print_table( print("bs active miss_rate misses time_ms copy_MiB bw_GBps tok_ms") print("-- ------ --------- ------ ------- -------- ------- ------") - cache = make_cache(profile, cache_slots, device) for batch_size in batch_sizes: for miss_rate in miss_rates: active, misses, time_ms = time_case( From 84109ada4b91ddbe8744012d45803a1bf58f6d43 Mon Sep 17 00:00:00 2001 From: Ruslan Rakhimov Date: Mon, 24 Aug 2026 20:58:36 +0300 Subject: [PATCH 2/2] fix(bench): report the skip in the exit status, and correct the comment Reworks the previous commit after review. A run that measured nothing still exited 0. print_table now reports whether it measured anything and main tallies the skips, following the shape bench_decode_moe already uses. The exit status distinguishes the two cases that are not alike: a derived default one backend cannot satisfy is expected output and stays 0, so the documented no-arg sweep is not red forever, while naming a geometry by hand and getting nothing -- or measuring nothing at all -- returns 1. The comment was wrong twice. The cap is 992 slots; "caps padded experts at 1024" is the constraint that makes 992 the limit, not the limit itself. And MARLIN_MAX_CACHE_SIZE is a public constant in the module this file already imports from, so "it would drift" was not the reason to catch rather than pre-check. The real reason is that the guard also covers the num_experts floor, so pre-checking would duplicate the rule rather than the number. Also notes that a device OOM is a RuntimeError and still aborts, which is right: it is not a statement about the geometry being illegal. --- benchmarks/bench_offload_cache_copy.py | 62 ++++++++++++++++++-------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/benchmarks/bench_offload_cache_copy.py b/benchmarks/bench_offload_cache_copy.py index 2fc99849..7bc227af 100644 --- a/benchmarks/bench_offload_cache_copy.py +++ b/benchmarks/bench_offload_cache_copy.py @@ -195,18 +195,24 @@ def print_table( miss_rates: list[float], repeat: int, device: torch.device, -) -> None: +) -> bool: + """False when the geometry is refused and nothing was measured; the caller tallies those.""" per_expert = expert_bytes(profile) cache_gib = cache_slots * per_expert / 2**30 - # Build before printing: a backend that refuses this slot count would otherwise leave a - # header with no rows under it, which is what the abort already looked like. The limit is - # the backend's (marlin caps padded experts at 1024) -- ask it by constructing rather than - # copying the number here, where it would drift the first time a backend changes. + # Build before printing: a refused geometry would otherwise leave a header with no rows + # under it, which is what the abort already looked like. `validate_rebuild` runs in + # __post_init__ ahead of the allocations, so the refused path costs nothing. + # + # Catch rather than pre-check: the rule is the pool's and covers both the num_experts + # floor and the nvfp4_marlin slot cap (992 -- moe_align_block_size needs + # round_up(experts, 32) < 1024). Re-deriving that here would duplicate the rule, not just + # the number. Note this is ValueError only: a device OOM is a RuntimeError and still + # aborts, which is correct -- it is not a statement about this geometry being illegal. try: cache = make_cache(profile, cache_slots, device) except ValueError as exc: - print(f"\n{name} @ cache_slots={cache_slots}: SKIPPED -- {exc}") - return + print(f"\n{name} @ cache_slots={cache_slots} ({cache_gib:.1f} GiB): SKIPPED -- {exc}") + return False print( f"\n{name} ({profile.quant_format}, {len(bank_specs(profile))} banks, " @@ -232,26 +238,44 @@ def print_table( ) del cache torch.cuda.empty_cache() + return True -def main() -> None: +def main() -> int: args = parse_args() assert torch.cuda.is_available(), "CUDA is required" torch.cuda.set_device(args.device) device = torch.device("cuda") print("gpu", torch.cuda.get_device_name(device), flush=True) - for name in args.models: - profile = MODELS[name] - slot_counts = args.cache_slots or [ - profile.experts, - int(0.4 * profile.layers * profile.experts), - ] - for cache_slots in slot_counts: - print_table( - name, profile, cache_slots, args.batch_sizes, args.miss_rates, args.repeat, device - ) + plan = [ + (name, MODELS[name], cache_slots) + for name in args.models + for cache_slots in ( + args.cache_slots + or [MODELS[name].experts, int(0.4 * MODELS[name].layers * MODELS[name].experts)] + ) + ] + skipped = [] + for name, profile, cache_slots in plan: + if print_table( + name, profile, cache_slots, args.batch_sizes, args.miss_rates, args.repeat, device + ): + continue + skipped.append(f"{name}@{cache_slots}") + if not skipped: + return 0 + + print(f"\nskipped {len(skipped)} of {len(plan)} combinations: {', '.join(skipped)}") + # A derived default that one backend cannot satisfy is expected output, not a failed + # run -- the no-arg sweep would otherwise be red forever. Asking for a geometry by hand + # and getting nothing is a different thing, and so is measuring nothing at all. + if len(skipped) == len(plan): + return 1 + if args.cache_slots is not None: + return 1 + return 0 if __name__ == "__main__": - main() + raise SystemExit(main())