[RLlib; Offline RL] 1. Fix multi-learner issue. #49194
Merged
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
…enerated. This produces otherwise an error, if the data is larger than in our sample files. Provided the Learner with an 'iterator' that keeps this iterator and iterates over it. Signed-off-by: simonsays1980 <[email protected]>
Signed-off-by: simonsays1980 <[email protected]>
simonsays1980
added
rllib
sven1977
reviewed
Dec 10, 2024
sven1977
reviewed
Dec 10, 2024
sven1977
reviewed
Dec 10, 2024
rllib/core/learner/learner.py
Outdated
| @@ -270,6 +270,9 @@ def __init__( | |||
| # and return the resulting (reduced) dict. | |||
| self.metrics = MetricsLogger() | |||
|
|
|||
| # TODO (simon): Describe for what we need this and define the type here. | |||
sven1977
reviewed
Dec 10, 2024
Signed-off-by: simonsays1980 <[email protected]>
simonsays1980
changed the title
8 tasks
sven1977
reviewed
Dec 11, 2024
| @@ -1088,6 +1094,9 @@ def update_from_iterator( | |||
| "`num_iters` instead." | |||
| ) | |||
|
|
|||
| if not self.iterator: | |||
There was a problem hiding this comment.
Dumb question: What if self.iterator is already set (to a previously incoming DataIterator)? Would the now-incoming iterator be thrown away?
There was a problem hiding this comment.
Yes, it would. But we want this: The incoming iterator might be different from the one assigned to the learner, but only because of ray.get not being able to call learners in order, i.e. the learners might get different streaming splits every training iteration - we do not want this to happen. One Learner same split.
sven1977
approved these changes
Dec 11, 2024
sven1977
added
rllib-newstack
and removed
go
ujjawal-khare
pushed a commit
to ujjawal-khare-27/ray
that referenced
this pull request
Dec 17, 2024
Signed-off-by: ujjawal-khare <[email protected]>
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?
Multi-learner setups were lately running with errors. The main reason was due to reinstantiation of a streaming split. This PR
OfflineData.sample.OfflineData.batch_iteratortoOfflineData.batch_iterators.OfflineData.samplemethod in general.learnerwith an additionaliteratorattribute that gets assigned to whenLearner.update_from_iteratoris called. This allows the Learner to pause the iterator in between training iterations and to use it again when the next iteration takes turn (notestreaming_splititerators are repeatable).**kwargstoLearner.update_from_batchesto improve customization and readability. This enables to call thelearner_group.updatemethod withkwargswhen in multi- OR in single-learner mode.Related issue number
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.