Repository navigation
[Feature] Auto-configure exploration module specs from environment in Collector - #3317
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/3317
Note: Links to docs will display an error until the docs builds have been completed. ❌ 4 New Failures, 1 Cancelled Job, 3 Pending, 1 Unrelated FailureAs of commit 52308fe with merge base 0a99aaf ( NEW FAILURES - The following jobs have failed:
CANCELLED JOB - The following job was cancelled. Please retry:
BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
vmoens
left a comment
There was a problem hiding this comment.
LGTM! Thanks so much for this.
Do you think we need similar edits for OU and EGreedy? I think Claude can do that easily.
I made the following changes if they're ok with you:
1. torchrl/modules/tensordict_module/exploration.py
a) Updated AdditiveGaussianModule docstring (lines 255-262):
- Changed
spec (TensorSpec)tospec (TensorSpec, optional) - Added explanation that it can be
Nonefor delayed initialization - Documented that spec must be set via property setter before calling
forward()
b) Added example to set_exploration_modules_spec_from_env (lines 822-840):
- Added a complete doctest example showing how to use the helper function
2. torchrl/modules/__init__.py
a) Added import (line 95):
from .tensordict_module.exploration import RandomPolicy, set_exploration_modules_spec_from_envb) Added to __all__ (line 186):
"set_exploration_modules_spec_from_env",3. docs/source/reference/modules_exploration.rst
Added new "Helpers" section (lines 17-24):
Helpers
-------
.. autosummary::
:toctree: generated/
:template: rl_template_noinherit.rst
set_exploration_modules_spec_from_env4. test/test_configs.py
Removed unnecessary hasattr checks (2 locations):
- Line ~1131-1134: Removed defensive
if hasattr(collector_instance, "shutdown"):check - Line ~1226-1228: Removed defensive
if hasattr(collector_instance, "shutdown"):check
Since shutdown is an abstract method in the base DataCollector class, these guards were unnecessary.
41aeb34 to
52308fe
Compare
Description
This PR enables the use of exploration modules (e.g.,
AdditiveGaussianModule) with the Hydra config system by supporting delayed spec initialization. When policies are instantiated via configs, the environment'saction_specis not available at construction time, causing exploration modules that require spec at init to fail. This change allowsspec=Noneat construction and auto-configures it when the collector is created.Motivation and Context
When using the trainer's config YAML framework, policies are instantiated by Hydra before the environment is created inside the collector. Exploration modules like
AdditiveGaussianModulepreviously required aspecargument at construction, which wasn't possible in this workflow since the spec depends on the environment.This PR does the following:
AdditiveGaussianModuleto support delayedspecsetting. With this change, you do not need to provide a spec at initialization, but you must provide it before callingforward().AdditiveGaussianModulespecon any exploration modulesTypes of changes
Checklist
Go over all the following points, and put an
xin all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!