Commit c8f79a7
committed
fix(customization): one metric store, so a withheld report cannot be erased
The callback kept two accumulators with opposite update rules. `_series`
merged by metric name; `_pending_metrics` held one whole withheld report in
a single slot and a second withheld report replaced it wholesale. That
asymmetry was the bug: the scalars in the displaced report were never sent
at all.
Two ways it bit, both reproduced before the fix. Across phases, a withheld
train step followed by a withheld validation pass -- the validation report
carries `val_` names, a key set disjoint from `train_`, so it carried
nothing fresher for what it displaced. Within one phase, two withheld
reports where the second omits a metric the first carried, which backends
do routinely. The sequence that hits it is the ordinary end of a run: last
train step, final validation, close().
Replaced both with one store keyed by qualified metric name, where the type
is the discriminator -- a list is a series and appends, a bare number is a
scalar and is replaced. Which one a name is gets decided once, the first
time it arrives, so the patterns are matched once per name instead of once
per name per step, and the "current value only" log needs no seen-set to
stay at one line per name. `_excluded_seen` drops out entirely.
Sending is now declarative: it transmits the state the server should hold
rather than replaying events since the last send. Scalars ride on every
report, which is what makes a dropped report self-healing -- `update_task`
swallows its failures, so a lost report used to leave a permanent hole.
Measured cost: train reports are unchanged at 8 scalars/206 bytes;
validation reports carry 12 instead of 4, +181 bytes against a series blob
already past 1.3 KB and growing. The series are still gated on having
changed, because they are the term that actually costs.
Two behaviours worth stating. A series still reports its newest point as a
top-level current value as well as its history -- being a series adds a
history, it does not cost the value. And a seeded series whose name the
current patterns would not select now resumes rather than freezing; no test
pinned the old behaviour, and the seeding exists so a process taking over
"continues its curves", which is what resuming does.
Signed-off-by: Albert Cui <albcui@nvidia.com>1 parent 3791855 commit c8f79a7
4 files changed
Lines changed: 258 additions & 131 deletions
File tree
- packages/nmp_customization_common
- src/nmp/customization_common/training
- tests/training
- services/automodel/tests/tasks/training/backends
0 commit comments