Unify sweep trees into conf/scaling/ + Python metric presets - #17
Open
philipnickel wants to merge 1 commit into
Open
philipnickel wants to merge 1 commit into
philipnickel wants to merge 1 commit into
Conversation
…c presets
Two readability/maintainability refactors against the Hydra/hydra-zen config
surface, verified to leave the composed configs unchanged.
1. conf/scaling/ axis group (single source of truth)
The swept sim.* values were duplicated across two parallel trees
(conf/sweep/* for timing, conf/nsight_sweep/* for NCU): the 9-value
particle list lived in 4 files, the ${ppc_grid:N} derivation in 2, the
density grid list in 2, the sm operating point in 2-3. They now live once
per axis in conf/scaling/{particle_count,weak,density,sm}.yaml; both wrapper
trees pull them via `defaults: - /scaling: <axis>` and keep only their
genuine differences (backend list +/- jax, plots vs nsight_plot callback,
output-dir roots).
2. Metric presets as a Python source of truth (nsight_metric_sets.py)
conf/nsight_metrics/{roofline,full}.yaml hand-duplicated the roofline
counters. FULL_METRICS is now composed as TIMING + ROOFLINE + extras, so the
roofline subset is defined once. Registered as the nsight_metrics config
group (package nsight.analyze), imported by profile_nsight.py before Hydra
composes; conf/nsight_metrics/ is removed.
Verification: --cfg job/hydra for all 9 sweep selections + 3 metric presets is
byte-identical before/after, except (a) the full preset's metric *order* (set
proven identical, 59 metrics, no dupes; order is irrelevant to NCU and the
long-format parquet) and (b) the expected new `scaling:` entry under
hydra.runtime.choices. 68 tests pass; ruff clean. Runtime-validated on an A100
across all five backends: benchmark, both timing sweeps (ppc_grid derivation
confirmed), and NCU collection (roofline 15/15, full 59/59) incl. an
nsight_sweep multirun with all five figures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two readability/maintainability refactors of the Hydra/hydra-zen config surface, verified to leave the composed configs unchanged.
1.
conf/scaling/axis group — single source of truthThe swept
sim.*values were duplicated across two parallel trees (conf/sweep/*for timing,conf/nsight_sweep/*for NCU):scaling/particle_count), shared${ppc_grid:…}derivationscaling/weak)scaling/density)scaling/sm)Both wrapper trees now pull the axis via
defaults: - /scaling: <axis>and keep only their genuine differences (backend list ±jax, plots vsnsight_plotcallback, output-dir roots).2. Metric presets as a Python source of truth —
nsight_metric_sets.pyconf/nsight_metrics/{roofline,full}.yamlhand-duplicated the roofline counters.FULL_METRICSis now composed asTIMING + ROOFLINE + extras, so the roofline subset is defined once. Registered as thensight_metricsconfig group (packagensight.analyze), imported byprofile_nsight.pybefore Hydra composes.conf/nsight_metrics/is removed.Verification
--cfg job/--cfg hydrafor all 9 sweep selections + 3 metric presets is byte-identical before/after, except:fullpreset's metric order (set proven identical: 59 metrics, no dupes — order is irrelevant to NCU and the long-format parquet);scaling:entry underhydra.runtime.choices.sim=benchmark, both timing sweeps (${ppc_grid:…}derivation confirmed: 100k→G=28, 200k→G=36), and NCU collection driven by the Python presets (roofline 15/15, full 59/59, zero missing), including annsight_sweepmultirun that produced all five figures.Follow-up (not in this PR)
P2 consistency pass: rename wrappers to the canonical axis names (
weak_scaling→weak,particle_density→density), hoist the duplicatedgpu_kindresolver, normalize indentation. Two stale.claude/CLAUDE.mdpointers toconf/nsight_metrics/should also be updated (the edit was auto-blocked as self-modification).🤖 Generated with Claude Code