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.

Sign up for GitHub

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

Jump to bottom

[RLlib] Restoring from a checkpoint from an older wheel (where AlgorithmConfig.rl_module_spec was NOT a @property yet) breaks when trying to load from this checkpoint in current version. #41157

Merged
merged 1 commit into from
Nov 15, 2023
Merged

[RLlib] Restoring from a checkpoint from an older wheel (where AlgorithmConfig.rl_module_spec was NOT a @property yet) breaks when trying to load from this checkpoint in current version. #41157

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions rllib/algorithms/algorithm_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -898,8 +898,8 @@ def validate(self) -> None:
"`config.rollouts(enable_connectors=True)`."
)

# Throw a warning if the user has used rl_module_spec but not enabled the
# new API stack.
# Throw a warning if the user has used `self.rl_module(rl_module_spec=...)` but
# has not enabled the new API stack at the same time.
if self._rl_module_spec is not None and not self._enable_new_api_stack:
logger.warning(
"You have setup a RLModuleSpec (via calling `config.rl_module(...)`), "
Expand All @@ -913,7 +913,8 @@ def validate(self) -> None:
setting_name="lr",
description="learning rate",
)

# Throw a warning if the user has used `self.training(learner_class=...)` but
# has not enabled the new API stack at the same time.
if self._learner_class is not None and not self._enable_new_api_stack:
logger.warning(
"You specified a custom Learner class (via "
Expand Down Expand Up @@ -2652,11 +2653,12 @@ def rl_module_spec(self):
default_rl_module_spec = self.get_default_rl_module_spec()
_check_rl_module_spec(default_rl_module_spec)

# `self._rl_module_spec` has been user defined (via call to `self.rl_module()`).
if self._rl_module_spec is not None:
# Merge provided RL Module spec class with defaults
_check_rl_module_spec(self._rl_module_spec)
# We can only merge if we have SingleAgentRLModuleSpecs.
# TODO(Artur): Support merging for MultiAgentRLModuleSpecs.
# TODO (sven): Support merging for MultiAgentRLModuleSpecs.
if isinstance(self._rl_module_spec, SingleAgentRLModuleSpec):
if isinstance(default_rl_module_spec, SingleAgentRLModuleSpec):
default_rl_module_spec.update(self._rl_module_spec)
Expand All @@ -2666,6 +2668,7 @@ def rl_module_spec(self):
"Cannot merge MultiAgentRLModuleSpec with "
"SingleAgentRLModuleSpec!"
)
# `self._rl_module_spec` has not been user defined -> return default one.
else:
return default_rl_module_spec

Expand Down Expand Up @@ -3472,6 +3475,11 @@ def __setattr__(self, key, value):
f"Cannot set attribute ({key}) of an already frozen "
"AlgorithmConfig!"
)
# Backward compatibility for checkpoints taken with wheels, in which
# `self.rl_module_spec` was still settable (now it's a property).
if key == "rl_module_spec":
key = "_rl_module_spec"

super().__setattr__(key, value)

def __getitem__(self, item):
Expand Down
Loading