-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[RLlib; Offline RL] Add tests for OfflineSingleAgentEnvRunner
.
#47133
[RLlib; Offline RL] Add tests for OfflineSingleAgentEnvRunner
.
#47133
Conversation
Signed-off-by: simonsays1980 <[email protected]>
Signed-off-by: simonsays1980 <[email protected]>
Signed-off-by: simonsays1980 <[email protected]>
OfflineSingleAgentEnvRunner
.OfflineSingleAgentEnvRunner
.
…to the docstrings in 'AlgorithmConfig' in regard to column compression. Signed-off-by: simonsays1980 <[email protected]>
@@ -1598,6 +1598,13 @@ py_test( | |||
srcs = ["offline/estimators/tests/test_dr_learning.py"], | |||
) | |||
|
|||
py_test( |
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.
awesome!
super nit: let's keep test cases alphabetically ordered (within their category), so let's move this below the next record (test_offline_data
).
@@ -2456,7 +2456,9 @@ def offline_data( | |||
to define the output data format when recording. | |||
input_compress_columns: What input columns are compressed with LZ4 in the | |||
input data. If data is stored in `RLlib`'s `SingleAgentEpisode` ( | |||
`MultiAgentEpisode` not supported, yet). | |||
`MultiAgentEpisode` not supported, yet). Note, | |||
`rllib.core.columns.Columns.OBS` will also try to decompress |
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.
Awesome. Thanks for clarifying this for the users. Makes perfect sense that they won't have to specify NEXT_OBS explicitly.
@@ -2513,7 +2515,8 @@ def offline_data( | |||
output_config: Arguments accessible from the IOContext for configuring | |||
custom output. | |||
output_compress_columns: What sample batch columns to LZ4 compress in the | |||
output data. | |||
output data. Note, `rllib.core.columns.Columns.OBS` will also compress |
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.
👍
@@ -193,7 +193,7 @@ def sample( | |||
getattr(samples_ds, self.output_write_method)( | |||
path.as_posix(), **self.output_write_method_kwargs | |||
) | |||
logger.info("Wrote samples to storage.") | |||
logger.info(f"Wrote samples to storage at {path}") |
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.
👍
@@ -250,7 +250,6 @@ def convert(sample, space): | |||
return sample | |||
|
|||
episodes = [] | |||
# TODO (simon): Give users possibility to provide a custom schema. |
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.
Nice!
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.
Awesome PR! Thanks for following up on these test cases @simonsays1980 .
Why are these changes needed?
This PR adds tests to check, if recording of agents works properly. Recording is managed by the
OfflineSingleAgentEnvRunner
that usesray.data
to write files to disk.This test suite tests:
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.