Skip to content

Commit

Permalink
Use override=True for config1 in adjust_config_by_multi_process_divid…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
emailweixu committed Dec 9, 2024
1 parent d30cdb4 commit 462a081
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
10 changes: 5 additions & 5 deletions alf/config_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def adjust_config_by_multi_process_divider(ddp_rank: int,
tag,
math.ceil(num_parallel_environments / multi_process_divider),
raise_if_used=False,
override_sole_init=True)
override_all=True)

# Adjust the mini_batch_size. If the original configured value is 64 and
# there are 4 processes, it should mean that "jointly the 4 processes have
Expand All @@ -167,7 +167,7 @@ def adjust_config_by_multi_process_divider(ddp_rank: int,
tag,
math.ceil(mini_batch_size / multi_process_divider),
raise_if_used=False,
override_sole_init=True)
override_all=True)

# If the termination condition is num_env_steps instead of num_iterations,
# we need to adjust it as well since each process only sees env steps taking
Expand All @@ -179,15 +179,15 @@ def adjust_config_by_multi_process_divider(ddp_rank: int,
tag,
math.ceil(num_env_steps / multi_process_divider),
raise_if_used=False,
override_sole_init=True)
override_all=True)

tag = 'TrainerConfig.initial_collect_steps'
init_collect_steps = get_config_value(tag)
config1(
tag,
math.ceil(init_collect_steps / multi_process_divider),
raise_if_used=False,
override_sole_init=True)
override_all=True)

# Only allow process with rank 0 to have evaluate. Enabling evaluation for
# other parallel processes is a waste as such evaluation does not offer more
Expand All @@ -197,7 +197,7 @@ def adjust_config_by_multi_process_divider(ddp_rank: int,
'TrainerConfig.evaluate',
False,
raise_if_used=False,
override_sole_init=True)
override_all=True)


def parse_config(conf_file, conf_params, create_env=True):
Expand Down
25 changes: 22 additions & 3 deletions alf/config_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,8 @@ def config1(config_name,
mutable=True,
raise_if_used=True,
sole_init=False,
override_sole_init=False):
override_sole_init=False,
override_all=False):
"""Set one configurable value.
Args:
Expand All @@ -391,15 +392,33 @@ def config1(config_name,
where the student must override certain configs inherited from the
teacher). If the config is immutable, a warning will be declared with
no changes made.
"""
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.)."""
config_node = _get_config_node(config_name)

if raise_if_used and config_node.is_used():
raise ValueError(
"Config '%s' has already been used. You should config "
"its value before using it." % config_name)

if override_sole_init:
if override_all:
if config_node.get_sole_init():
logging.warning(
"The value of config '%s' (%s) is protected by sole_init. "
"It is now being overridden by the overide_all flag to a new value %s. "
"Use at your own risk." % (config_name,
config_node.get_value(), value))
if not config_node.is_mutable():
logging.warning(
"The value of config '%s' (%s) is immutable. "
"It is now being overridden by the overide_all flag to a new value %s. "
"Use at your own risk." % (config_name,
config_node.get_value(), value))
config_node.set_value(value)
return
elif override_sole_init:
if config_node.is_configured():
if not config_node.is_mutable():
logging.warning(
Expand Down

0 comments on commit 462a081

Please sign in to comment.