Skip to content

fix(test): increase model dim for stable timing benchmark - #84

Closed
cennn wants to merge 1 commit into
mainfrom
fix/stable-timing-benchmark
Closed

cennn wants to merge 1 commit into
mainfrom
fix/stable-timing-benchmark

Conversation

@cennn

@cennn cennn commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator

Bump SimpleModel dim 128→1024 and seq_len 256→512 in test_simple_model_timing_class_function_instance_method so GPU compute dominates over kernel-launch overhead.

Problem: With dim=128, timings are ~0.2ms where kernel launch jitter causes 4-6x ratio swings, making the max/min < 1.2 assertion flaky on fast GPUs (B300/H100).

Fix: Larger model → ~0.24ms median with stdev ~0.005ms → ratio stable at ~1.02.

Tested on B300 (SM103): all 4 entry points converge to 0.238-0.244ms median.

Bump SimpleModel dim 128→1024 and seq_len 256→512 so GPU compute
dominates over kernel-launch overhead, eliminating flaky ratio
assertions on fast GPUs (B300/H100).
@cennn

cennn commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #89 (rebased the still-useful parts onto current main). Closing this PR.

@cennn cennn closed this Oct 14, 2026
jiahy0825 pushed a commit that referenced this pull request Oct 15, 2026
…split graphs (#89)

* fix: cache key on graph+placements, AOT JIT fallback, skip FSDP overlap on split graphs

Unmerged remnants of #88 / #86 / #84 / #79, rebased onto current main:

- Cache key includes graph.code and DTensor placements. Changing SimpleFSDP
  fully_shard -> replicate used to reuse the old artifact (1-rank AllGathers
  served as DtoD copies).
- AOT compile failures fall back to JIT so MAGI_COMPILE_AOT=1 can stay
  process-wide (VAE AOT, SimpleFSDP DiT JIT).
- enable_fsdp no longer runs fullgraph overlap when disable_graph_split is
  False (VAE in a monolithic FSDP process). #79's restart-analysis half
  already landed as #75.
- Timing bench dim 128->1024 so launch jitter does not flake max/min < 1.2
  on B300/H100.

* fix(cache): hash input placements only, not graph.code

Restart-analysis retraces with different FX node names, so including graph.code
in the magi cache key made the second pass miss the first-pass directory
(test_assert_cache_hit_succeeds_with_restart_analysis / test_restart_analysis_cache
on pt212). DTensor fully_shard vs replicate is still distinguished by the input
placement signature.

* fix(cache): hash only DTensor placements so restart-analysis still hits

* fix(aot): do not swallow AssertionError/OOM when AOT falls back to JIT
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.

1 participant