Skip to content

Fix AdvancedProfiler.__reduce__ dump_stats state loss and SimpleProfiler(extended=False) empty summary crash - #22053

Open
deep1blue wants to merge 1 commit into
Lightning-AI:masterfrom
deep1blue:fix/profiler-reduce-and-empty-summary
Open

deep1blue wants to merge 1 commit into
Lightning-AI:masterfrom
deep1blue:fix/profiler-reduce-and-empty-summary

Conversation

@deep1blue

Copy link
Copy Markdown

What does this PR do?

Fixes two self-discovered profiler edge-case bugs in lightning.pytorch.profilers:

  1. AdvancedProfiler.__reduce__ silently resets dump_stats=True to False (src/lightning/pytorch/profilers/advanced.py):

    • AdvancedProfiler.__reduce__ returns (self.__class__, (), {...}) so cProfile.Profile objects do not raise TypeError during pickling/deepcopy (e.g., when cloning the trainer/callbacks or spawning worker processes).
    • Because __init__ is invoked with () (where dump_stats=False by default) and the returned state dictionary omitted "dump_stats": self.dump_stats, any copy.deepcopy(profiler) or pickle.loads(pickle.dumps(profiler)) silently reset profiler.dump_stats from True back to False, preventing .prof files from being written in describe().
    • Adding "dump_stats": self.dump_stats to the __reduce__ state dictionary preserves dump_stats across pickling and deepcopy.
  2. SimpleProfiler(extended=False).summary() raises ValueError: max() iterable argument is empty when no actions have been recorded (src/lightning/pytorch/profilers/simple.py):

    • In SimpleProfiler.summary(), the if self.extended: branch guards max(len(k) for k in self.recorded_durations) with if len(self.recorded_durations) > 0:, returning an empty report header when called before any actions are recorded.
    • However, the else: branch (extended=False) omitted that guard and unconditionally called max(len(k) for k in self.recorded_durations), raising ValueError: max() iterable argument is empty if summary() or describe() is called on an empty SimpleProfiler(extended=False).
    • Replacing else: with elif len(self.recorded_durations) > 0: makes SimpleProfiler(extended=False) consistent with SimpleProfiler(extended=True).

Testing

pytest tests/tests_pytorch/profilers/test_profiler.py -k "simple_profiler or advanced_profiler" -v
# 37 passed, 1 skipped

Acknowledgements

Credit to the Lightning AI team and contributors (@Borda, @carmocca, @awaelchli, @ajstewart, @bhimrajyadav) for the design and maintenance of SimpleProfiler and AdvancedProfiler.

…ler(extended=False) empty summary crash

Signed-off-by: Parth Vadera <parthvadera@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant