Skip to content

fix: avoid mutable default arguments in exp_manager timer callbacks - #15952

Open
andrewwhitecdw wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:fix-mutable-default-timer-kwargs
Open

fix: avoid mutable default arguments in exp_manager timer callbacks#15952
andrewwhitecdw wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:fix-mutable-default-timer-kwargs

Conversation

@andrewwhitecdw

Copy link
Copy Markdown
Contributor

Replaces mutable default arguments {} with None in TimingCallback and DeltaTimingCallback, then instantiates a fresh empty dict inside each constructor. This prevents shared state across instances if the dict is ever mutated.

  • nemo/utils/exp_manager.py: changed timer_kwargs={} to timer_kwargs: Optional[dict] = None for both callbacks
  • Added regression tests verifying fresh dicts are passed and the default is None

Tested:

uv run python -m pytest tests/core/test_exp_manager_timer_callbacks.py -v
# 2 passed

cc @nithinraok for core review

Signed-off-by: Andrew White <andrewh@cdw.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added core Changes to NeMo Core community-request labels Jul 24, 2026
@andrewwhitecdw

Copy link
Copy Markdown
Contributor Author

Could a maintainer please add the Run CICD label to trigger CI? As an outside contributor I don't have permission to apply labels myself. Thanks!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove test

@svcnvidia-nemo-ci svcnvidia-nemo-ci added the waiting-on-customer Waiting on the original author to respond label Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-request core Changes to NeMo Core waiting-on-customer Waiting on the original author to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants