refactor: derive metrics, objectives, and run artifacts from one registry - #23
Merged
Conversation
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.
The NSGA selection machinery was already arity-agnostic, but the pipeline feeding it was not: adding a metric meant editing 53 sites across 9 files, and the README's extensibility claim had to be hedged because of it. Now that count is zero, and the claim is verified rather than asserted.
METRICSinscore/complexity.pymaps a metric name to a measure over(rendered PNG, source text). Everything downstream derives from it:SearchNode/Resultcarry ametricsdict instead of named fields,build_objectivesemits(score, *METRIC_NAMES),LINEAGE_COLUMNSsplices the metric names into the header, and both analysis scripts read their columns, Pareto keys, plot axis, and summary table from the registry.Visual error stays deliberately outside it: it comes from the configured scorer, not a complexity measure, and it is the constraint-gated primary objective rather than one of the interchangeable tiebreakers.
Verified by actually adding a fourth metric. One entry, no other edits, then observed it propagate:
Reverted afterwards; a permanent test now pins the properties instead (registry contents,
measure_allcovering every metric, lineage columns containing every metric, and objective arity equal tolen(METRIC_NAMES) + 1).resume.py's positional 7-tuple is gone, replaced by aPreppedNodedataclass. This was the worst of the 53 sites: metrics were read asprepped_nodes[i][4]and[i][5], so registering a metric would have silently shifted every later index. That is the same misalignment class already fixed forstats.csvandlineage.csv, and it was the last instance.The round-trip test added in #22 caught a bug I introduced here. Generalizing
load_complexities_from_lineagetonode.update(read_metrics(row))dropped an implicit guard: the oldif row.get("visual_complexity")check also skipped sparse eviction rows. Without it, an eviction row zeroed the metrics its node's real row had already supplied — node 2 read back0.0instead of2000.0. Fixed with an explicitrow_has_metricspredicate, which is clearer about why the check exists. Worth noting because the previous suite would not have caught this.read_metricsdefaults absent columns to0.0, so a row written before a metric was registered stays readable, and the pre-split legacycomplexitycolumn mapping now lives next to the registry instead of being restated in both scripts.The plot axis and summary table are registry-driven too: the scatter plots the first registered metric with a label derived from its name, and the star annotation reports the true objective count. The Pareto front is still a 2-D projection of an N-objective front, which the docstring already explains.
README updated: the extensibility paragraph no longer hedges, and it now points at the registry and explains why score sits outside it. The prior warning about dominance diluting past ~4 objectives is retained and repeated as a comment on the registry, since making objectives cheap to add is not the same as making them free.
Verified end to end after the refactor: all three formats complete an offline multi-epoch search with
--tournament-size 3and--epoch-variance 0.02(svg 0.0027, graphviz 0.1467, typst 0.0586), the lineage header and rows are correct, and both scripts run clean against a real run.459 passed, 13 skipped, ruff clean.