[RLlib] Add functionality to define kernel and bias initializers for RLModules.
#42137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…etworks. This commit implements it for framework 'tf2'. Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
… the initial dense layer of 'TransposeCNNHead'. Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
sven1977
changed the title
RLModules.RLModules.
sven1977
reviewed
Jan 5, 2024
sven1977
reviewed
Jan 5, 2024
sven1977
reviewed
Jan 5, 2024
|
|
||
| # Initialize, if necessary. | ||
| if cnn_transpose_initializer: | ||
| if cnn_transpose_initializer_config: |
There was a problem hiding this comment.
Simplify to:
if cnn_transpose_initializer_config is not None:
cnn_transpose_initializer(
layer.weight, **(cnn_transpose_initializer_config or {})
)
Gets rid of one entire if-else-block.
There was a problem hiding this comment.
Great catch! Thanks @sven1977 !
There was a problem hiding this comment.
I think we do not need the or {} there
sven1977
reviewed
Jan 5, 2024
| bias=cnn_transpose_use_bias or is_final_layer, | ||
| ), | ||
|
|
||
| layer = nn.ConvTranspose2d( |
sven1977
reviewed
Jan 5, 2024
| ) | ||
| if cnn_initializer: | ||
| if cnn_initializer_config: |
There was a problem hiding this comment.
Same simplification as described below (see Conv Transpose).
sven1977
reviewed
Jan 5, 2024
| output_initializer(layer.weight) | ||
| else: | ||
| if hidden_initializer: | ||
| if hidden_layer_initializer_config: |
There was a problem hiding this comment.
Same simplification as described below (see Conv Transpose).
sven1977
reviewed
Jan 5, 2024
rllib/core/models/torch/heads.py
Outdated
|
|
||
| if initial_dense_initializer: | ||
| if config.initial_dense_initializer_config: | ||
| initial_dense_initializer( |
There was a problem hiding this comment.
Same simplification as described below (see Conv Transpose).
…review from @sven1977. Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
…does not allow like the others None for the initializer. Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
sven1977
requested review from
avnishn,
ArturNiederfahrenhorst,
smorad,
maxpumperla and
kouroshHakha
as code owners
sven1977
reviewed
Jan 9, 2024
Signed-off-by: Sven Mika <sven@anyscale.io>
sven1977
approved these changes
Jan 9, 2024
There was a problem hiding this comment.
LGTM. Pending test cases for merge ... Thanks @simonsays1980 !
|
Hey @simonsays1980 , could you check the tests? I think 2 are failing due to the new changes. |
… were not yet named right. Also added docs for these. Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@sven1977 There were a couple of minor bugs yesterday - the ones you saw after my last push. Hopefully with my commit today tests should all pass. |
simonsays1980
changed the title
RLModules.RLModules.
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
vickytsang
pushed a commit
to ROCm/ray
that referenced
this pull request
Jan 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
The new stack does not offer users to define the initializers for network weights. Following literature (e.g. https://arxiv.org/abs/2006.05990) initialization of network weights can have signficant influence on policy performance.
This PR adds the funcitonality of network initialization to the new stack and enables thereby users to
Related issue number
Closes #40259
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.