Skip to content

Fix mutable default in Megatron init and IndexError on empty ModuleList#3944

Merged
SunMarc merged 2 commits intohuggingface:mainfrom
jashshah999:bugfix-megatron-default-and-empty-modulelist
Mar 2, 2026
Merged

Fix mutable default in Megatron init and IndexError on empty ModuleList#3944
SunMarc merged 2 commits intohuggingface:mainfrom
jashshah999:bugfix-megatron-default-and-empty-modulelist

Conversation

@jashshah999
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes two bugs:

  1. utils/megatron_lm.py - initialize() used args_defaults={} as a default parameter. Mutable defaults are shared across calls, so any mutation persists into subsequent invocations.

  2. utils/other.py - is_repeated_block() accessed module[0] without checking if the ModuleList was empty. An empty ModuleList would raise IndexError before the all() check could short-circuit.

… ModuleList

In megatron_lm.py, initialize() used args_defaults={} as a default
parameter, which is shared across calls and can cause unexpected
state persistence.

In other.py, is_repeated_block() accessed module[0] without checking
if the ModuleList was empty, causing IndexError for empty ModuleLists.
Copy link
Copy Markdown
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

Thanks

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@SunMarc
Copy link
Copy Markdown
Member

SunMarc commented Mar 2, 2026

@bot /style

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 2, 2026

Style bot fixed some files and pushed the changes.

@SunMarc SunMarc merged commit 7554afb into huggingface:main Mar 2, 2026
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.

3 participants