[Data] Fix Timer JSON serialization broken by DistributionTracker#63825
Merged
elliot-barn merged 2 commits intoJun 3, 2026
Merged
Conversation
…ngest benchmark PR ray-project#63586 added a DistributionTracker to Timer for percentile tracking. DistributionTracker is not JSON-serializable, which broke the training-ingest benchmark: its checkpointing code dumps each Timer via `v.__dict__.copy()` + `json.dump`, raising `TypeError: Object of type DistributionTracker is not JSON serializable`. (The tracker's __getstate__/__setstate__ only cover pickle, not json.) Add `Timer.as_dict()` / `from_dict()` that round-trip only the scalar fields (the sketch is not persisted; it restarts empty on restore, same graceful degradation as when datasketches is unavailable), and switch the benchmark runner to use them instead of poking `__dict__`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: xgui <xgui@anyscale.com>
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces as_dict and from_dict methods to safely serialize and deserialize scalar stats, resolving JSON serialization issues caused by the non-serializable DistributionTracker in the benchmark runner. Direct __dict__ access has been replaced with these new methods, and comprehensive unit tests have been added. The review feedback recommends improving the robustness of from_dict by adding defensive checks to handle cases where the input state is None, not a dictionary, or contains None values for _total and _total_count.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Ignore a non-dict ``state`` and treat explicit ``None`` values as the empty-Timer defaults (``dict.get(k, 0)`` only defaults a missing key, not a present ``None``), so a partial/corrupt metrics.json can't crash or leave a Timer in an invalid state on restore. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: xgui <xgui@anyscale.com>
liulehui
approved these changes
Jun 3, 2026
rueian
pushed a commit
to rueian/ray
that referenced
this pull request
Jun 4, 2026
…y-project#63825) ## Why ray-project#63586 added a `DistributionTracker` to `Timer` for percentile tracking. `DistributionTracker` is not JSON-serializable, which broke the `training-ingest-benchmark` release test (`image_classification.full_training`, s3_url + parquet): ``` TypeError: Object of type DistributionTracker is not JSON serializable ``` The benchmark checkpoints each `Timer` via `v.__dict__.copy()` + `json.dump`, and `__dict__` now carries the tracker. The tracker's `__getstate__`/`__setstate__` only cover **pickle**, not `json`, so they don't help here. ## Fix - Add `Timer.as_dict()` / `Timer.from_dict()` that round-trip only the scalar fields (`_total`, `_min`, `_max`, `_total_count`). `_distribution` is excluded — the KLL sketch isn't reconstructable from summary stats and isn't meant to persist across checkpoints, so it restarts empty on restore (same graceful degradation as when `datasketches` isn't installed). - Switch the benchmark runner to use these instead of poking `__dict__`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: xgui <xgui@anyscale.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
limarkdcunha
pushed a commit
to limarkdcunha/ray
that referenced
this pull request
Jun 30, 2026
…y-project#63825) ## Why ray-project#63586 added a `DistributionTracker` to `Timer` for percentile tracking. `DistributionTracker` is not JSON-serializable, which broke the `training-ingest-benchmark` release test (`image_classification.full_training`, s3_url + parquet): ``` TypeError: Object of type DistributionTracker is not JSON serializable ``` The benchmark checkpoints each `Timer` via `v.__dict__.copy()` + `json.dump`, and `__dict__` now carries the tracker. The tracker's `__getstate__`/`__setstate__` only cover **pickle**, not `json`, so they don't help here. ## Fix - Add `Timer.as_dict()` / `Timer.from_dict()` that round-trip only the scalar fields (`_total`, `_min`, `_max`, `_total_count`). `_distribution` is excluded — the KLL sketch isn't reconstructable from summary stats and isn't meant to persist across checkpoints, so it restarts empty on restore (same graceful degradation as when `datasketches` isn't installed). - Switch the benchmark runner to use these instead of poking `__dict__`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: xgui <xgui@anyscale.com> 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
3 participants
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.
Why
#63586 added a
DistributionTrackertoTimerfor percentile tracking.DistributionTrackeris not JSON-serializable, which broke thetraining-ingest-benchmarkrelease test (image_classification.full_training, s3_url + parquet):The benchmark checkpoints each
Timerviav.__dict__.copy()+json.dump, and__dict__now carries the tracker. The tracker's__getstate__/__setstate__only cover pickle, notjson, so they don't help here.Fix
Timer.as_dict()/Timer.from_dict()that round-trip only the scalar fields (_total,_min,_max,_total_count)._distributionis excluded — the KLL sketch isn't reconstructable from summary stats and isn't meant to persist across checkpoints, so it restarts empty on restore (same graceful degradation as whendatasketchesisn't installed).__dict__.🤖 Generated with Claude Code