{{ message }}
Repository navigation
[distributed][tools] Fix MemoryTracker.load not restoring the operation count - #192476
Closed
ousamabenyounes wants to merge 1 commit into
Closed
ousamabenyounes wants to merge 1 commit into
ousamabenyounes wants to merge 1 commit into
Conversation
pytorch-bot
Bot
added
ciflow/h100-symm-mem
ciflow/torchtitan
|
|
|
|
|
The following ciflow label(s) have been added but CI has not been triggered yet because the workflows are awaiting approval:
Once a maintainer approves the workflows (scroll to the bottom of the PR page), the corresponding CI jobs will be triggered automatically. Please ping one of the reviewers if you do not have access to approve and run workflows. |
drisspg
added
triaged
ousamabenyounes
force-pushed
the
fix/issue-191397
branch
from
August 16, 2026 16:53
5d9a5e3 to
3991310
Compare
Member
|
@pytorchbot merge |
Member
|
@pytorchbot merge -i |
Collaborator
Merge startedYour change will be merged while ignoring the following 4 checks: BC Lint, docs-build, pull, torchtitan-test / torchtitan-x-pytorch-test / test (torchtitan_models_integration, 1, 1, mt-l-x86aavx2-189-704-a10g-8) Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Collaborator
Merge failedReason: Command Details for Dev Infra teamRaised by workflow job |
save_stats() persisted the memory traces but not _op_index, and load() restored the traces without reconstructing it. summary() and show_traces() bound their iteration with _op_index, so after a save/load round trip the loaded tracker reported no operator deltas even though the trace entries were present. Persist op_index in save_stats() and restore it in load(), falling back to the number of recorded operations for stats files written before op_index was saved. Add a CPU regression test asserting the restored op count and that summary() still emits the per-operator deltas. Fixes pytorch#191397
d4l3k
force-pushed
the
fix/issue-191397
branch
from
October 2, 2026 02:16
3991310 to
58c640c
Compare
Member
|
@pytorchbot merge |
Collaborator
pytorchmergebot
pushed a commit
that referenced
this pull request
Oct 2, 2026
…on count (#192476) Fixes #191397 ### Problem `MemoryTracker.save_stats()` did not persist `_op_index`, and `load()` did not restore it. After `load()`, `_op_index` stayed `0`, so `summary()`/`show_traces()` (which iterate `range(1, self._op_index)`) produced no output on a loaded tracker. ### Fix Persist `op_index` in `save_stats()` and restore it in `load()`, with a backward-compatible fallback (`stats.get("op_index", len(self.memories_allocated))`) for stats files written before this key existed. ### Test verification (RED → GREEN) New test `test_save_load_preserves_op_index`: ``` # RED (unmodified main): FAILED test_save_load_preserves_op_index # GREEN (with fix): 1 passed ``` > **Disclosure (per [AI_POLICY.md](https://github.com/pytorch/pytorch/blob/main/AI_POLICY.md)):** this change was prepared with AI assistance and was reviewed, tested, and is submitted by me. Happy to iterate on any feedback. Pull Request resolved: #192476 Approved by: https://github.com/d4l3k
cyyever
pushed a commit
to BakeLab/pytorch
that referenced
this pull request
Oct 11, 2026
…on count (pytorch#192476) Fixes pytorch#191397 ### Problem `MemoryTracker.save_stats()` did not persist `_op_index`, and `load()` did not restore it. After `load()`, `_op_index` stayed `0`, so `summary()`/`show_traces()` (which iterate `range(1, self._op_index)`) produced no output on a loaded tracker. ### Fix Persist `op_index` in `save_stats()` and restore it in `load()`, with a backward-compatible fallback (`stats.get("op_index", len(self.memories_allocated))`) for stats files written before this key existed. ### Test verification (RED → GREEN) New test `test_save_load_preserves_op_index`: ``` # RED (unmodified main): FAILED test_save_load_preserves_op_index # GREEN (with fix): 1 passed ``` > **Disclosure (per [AI_POLICY.md](https://github.com/pytorch/pytorch/blob/main/AI_POLICY.md)):** this change was prepared with AI assistance and was reviewed, tested, and is submitted by me. Happy to iterate on any feedback. Pull Request resolved: pytorch#192476 Approved by: https://github.com/d4l3k
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.
Fixes #191397
Problem
MemoryTracker.save_stats()did not persist_op_index, andload()did not restore it. Afterload(),_op_indexstayed0, sosummary()/show_traces()(which iteraterange(1, self._op_index)) produced no output on a loaded tracker.Fix
Persist
op_indexinsave_stats()and restore it inload(), with a backward-compatible fallback (stats.get("op_index", len(self.memories_allocated))) for stats files written before this key existed.Test verification (RED → GREEN)
New test
test_save_load_preserves_op_index: