Sync metric docstrings with the keys actually returned#86
Open
lonexreb wants to merge 1 commit intoNVlabs:mainfrom
Open
Sync metric docstrings with the keys actually returned#86lonexreb wants to merge 1 commit intoNVlabs:mainfrom
lonexreb wants to merge 1 commit intoNVlabs:mainfrom
Conversation
The docstrings in src/alpamayo_r1/metrics/metric_utils.py and
src/alpamayo_r1/metrics/distance_metrics.py advertise return-dict keys
that are never produced:
- summarize_metric() docstring lists `name_sq`, but the body only adds
`name_std` (mean_sq is computed as an intermediate of std and not
stored). Also clarifies that `_std` is added only when N > 1, which
is not obvious from the current wording.
- compute_minade() docstring lists `min_ade_sq`, but the body returns
`min_ade` plus per-horizon keys of the form
`min_ade/by_t={t * time_step:.1f}` (and corresponding `_std` keys
via summarize_metric). The per-horizon keys are the headline output
of the function and were undocumented.
- compute_grouped_corner_distance() docstring lists
`corner_distance_sq`, but the body returns `corner_distance` (and
`corner_distance_std` from summarize_metric when N > 1).
This commit aligns the docstrings with the actual returned keys and
documents how summarize_metric layers `_std` on top. Pure docs change.
Signed-off-by: lonexreb <[email protected]>
This was referenced May 4, 2026
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.
Problem
The docstrings in
src/alpamayo_r1/metrics/metric_utils.pyandsrc/alpamayo_r1/metrics/distance_metrics.pyadvertise return-dict keys that are never produced. Anyone iterating these dicts looking for the documented_sqkeys gets aKeyError.1.
summarize_metric(metric_utils.py)Docstring claims it returns:
But the body only adds
name_std:mean_sqis computed as an intermediate ofstdand discarded. Also, the gating onN > 1is not obvious from the docstring.2.
compute_minade(distance_metrics.py)Docstring lists
min_ade_sq, but the body returnsmin_adeplus per-horizon keys of the formmin_ade/by_t={t * time_step:.1f}(then optionally_stdvariants viasummarize_metric). The per-horizon keys are the function's headline output and were entirely undocumented.3.
compute_grouped_corner_distance(distance_metrics.py)Docstring lists
corner_distance_sq, but the body returnscorner_distance(andcorner_distance_stdfromsummarize_metricwhenN > 1anddisable_summary=False).Fix
Pure docstring change — no code logic touched. Aligns each docstring with the actual returned keys and documents how
summarize_metriclayers_stdon top.Verification
Skimmed the call sites (none rely on the documented
_sqkeys), and the new wording matches the runtime behavior verifiable by reading the function bodies.