Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use override=True for config1 in adjust_config_by_multi_process_divid… #1714

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

emailweixu
Copy link
Contributor

…er()

Sometimes, those configurations are provided through commandline. In general, configurations provided through commandline will not be changed by config1(). However, for DDP training, even if these configurations are from commandline, we still need to change them.

…er()

Sometimes, those configurations are provided through commandline. In general,
configurations provided through commandline will not be changed by config1().
However, for DDP training, even if these configurations are from commandline,
we still need to change them.
@emailweixu emailweixu requested a review from QuantuMope December 9, 2024 22:10
QuantuMope
QuantuMope previously approved these changes Dec 10, 2024
Copy link
Contributor

@QuantuMope QuantuMope left a comment

Choose a reason for hiding this comment

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

LGTM

override_all (bool): If True, the value of the config will be set regardless
of any pre-existing ``mutable`` or ``sole_init`` settings. This should
be used only when absolutely necessary (e.g., adjusting certain configs
such as mini_batch_size for DDP workers.)."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting Nit: Add a newline before """

@emailweixu emailweixu merged commit e21e694 into pytorch Dec 11, 2024
2 checks passed
@emailweixu emailweixu deleted the PR_fix_adjust_config_by_multi_process_divider branch December 11, 2024 20:17
qiang-sean-liu pushed a commit that referenced this pull request Jan 14, 2025
#1714)

* Use override=True for config1 in adjust_config_by_multi_process_divider()

Sometimes, those configurations are provided through commandline. In general,
configurations provided through commandline will not be changed by config1().
However, for DDP training, even if these configurations are from commandline,
we still need to change them.

* Fix doc-string format
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.

2 participants