-
Notifications
You must be signed in to change notification settings - Fork 243
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
Feature/param reset #328
base: master
Are you sure you want to change the base?
Feature/param reset #328
Conversation
…impl. Also 1.Updated the conservative loss of discrete cql to be captured including the alpha multiplication to align with continuous cql. 2. Updated the critic loss of ddpg and continuous CQL to use dataclasses - aligning with DQN and discrete cql
@takuseno I am still working on the tests but please let me know if you think the implementation is a reasonable approach |
def _get_layers(self, q_func:nn.ModuleList)->List[nn.Module]: | ||
all_modules = {nm:module for (nm, module) in q_func.named_modules()} | ||
q_func_layers = [ | ||
*all_modules["_encoder._layers"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@takuseno assuming you're happy with the general approach of using the epoch_callback to inject the parameter reset functionality - I wondered if you could recommend a better approach for obtaining the encoder and fc layers which follows static typing?
@joshuaspear Thanks for the proposal! For now, can we make this as an experimental feature? I imagine something like this: file locationLet's make
usageJust rough illustration:
In this way, we can use the existing Reset is still under investigation in RL community. Once it gets more mature, we can lift this from |
Makes sense - will have a go next week :) |
Closes #326