Emit kernel-aware NAS JSON artifacts#108
Conversation
📝 WalkthroughWalkthroughThis PR adds JSON report output to the NAS experiment script. It introduces report-building functions to generate deterministic latency predictions and rankings across hardware targets, integrates them into the CLI with a ChangesNAS JSON Report Output
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/nas_experiment.py (1)
298-298: ⚡ Quick winConsider building the report only when needed.
The report is built unconditionally at line 298 but only used when
--json-outputis provided. Moving thebuild_report()call inside the conditional block would avoid unnecessary computation when users don't request JSON output.♻️ Proposed refactor
model = ArchitectureCostModel(hardware=args.hardware) -report = build_report(model) run_comparison(model) run_kernel_cliff_test(model) print(f"\n{'='*90}") print(" Key takeaways:") print(" - MLP (GeGLU) dominates for large models; attention dominates for long seq + many heads") print(" - Tile-unaligned dims (not multiple of 128) pay a measurable penalty") print(" - GQA (low num_kv_heads) saves attention time but QKV projection is still large") print(" - NAS should prefer hidden_dim, ffn_dim that are multiples of 128 (or 256)") print(f"{'='*90}\n") if args.json_output: + report = build_report(model) write_report(report, args.json_output) print(f"Wrote JSON artifact: {args.json_output}")Also applies to: 311-313
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/nas_experiment.py` at line 298, The call to build_report(model) is executed unconditionally and should be moved inside the branch that handles the JSON output flag to avoid wasted work; modify the code so build_report(model) is only invoked when the JSON output flag (e.g., args.json_output or the '--json-output' branch) is true, create the report variable inside that conditional, and update the blocks that currently reference report (lines ~311-313) to use that locally-built report instead of expecting a prebuilt report.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/nas_experiment.py`:
- Line 298: The call to build_report(model) is executed unconditionally and
should be moved inside the branch that handles the JSON output flag to avoid
wasted work; modify the code so build_report(model) is only invoked when the
JSON output flag (e.g., args.json_output or the '--json-output' branch) is true,
create the report variable inside that conditional, and update the blocks that
currently reference report (lines ~311-313) to use that locally-built report
instead of expecting a prebuilt report.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d3febfb-4551-499c-9f98-5032eceacede
📒 Files selected for processing (3)
docs/research/kernel-aware-nas.mdscripts/nas_experiment.pytests/test_arch_cost_model.py
Summary
Refs #81
Tests
Note: ruff is not installed locally.
Summary by CodeRabbit
New Features
--json-outputflag to generate machine-readable JSON reports containing latency predictions, performance rankings, timing breakdowns, and summary metrics for benchmark comparisons.Documentation