[RLlib] Stats bug fix: EMA stats w/o window would lead to infinite list mem-leak. #45752
Conversation
sven1977
requested review from
ArturNiederfahrenhorst and
simonsays1980
as code owners
sven1977
added
the
tests-ok
| @@ -310,7 +310,7 @@ def __call__( | |||
| ) | |||
| self.add_n_batch_items( | |||
| batch=data, | |||
| column="loss_mask", | |||
| column=Columns.LOSS_MASK, | |||
There was a problem hiding this comment.
Great! More constants! This will reduce our errors everywhere in the lib.
| None make them synchronous calls. | ||
| timeout_seconds: Time to wait (in seconds) for results. Set this to 0.0 for | ||
| fire-and-forget. Set this to None (default) to wait infinitely (i.e. for | ||
| synchronous execution). |
|
|
||
| Returns: | ||
| A tuple of corresponding (remote_calls, remote_actor_ids, valid_tags) | ||
|
|
||
| A tuple consisting of a list of the remote calls that match the tag(s), |
There was a problem hiding this comment.
Interesting. Did we have this all this time? So we can tag certain tasks and check, if these tasks have been worked?
There was a problem hiding this comment.
Correct. This was always there, albeit rarely used b/c we don't really have any async algos on Learner API, yet.
You can send async requests to the ActorManager with a tag, then - later - fetch the async results from the manager using this tag, kind of as a label to say: I only want these results, the others - even if already ready - I don't care about right now and will fetch them later.
| ) | ||
| remote_calls = [] | ||
| remote_actor_ids = [] | ||
| valid_tags = [] | ||
| for call, (tag, actor_id) in self._in_flight_req_to_actor_id.items(): | ||
| # the default behavior is to return all ready results. | ||
| if not len(tags) or tag in tags: | ||
| if len(tags) == 0 or tag in tags: |
There was a problem hiding this comment.
Does this mean we throw away all other results not having this tag? What if we have e.g. 2 tags and we call this function and we need afterwards the two tags separately (maybe results are from different sampling regimes) - cann we distinguish by gthe valid_tags the results?
…st mem-leak. (ray-project#45752) Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
…st mem-leak. (ray-project#45752) Signed-off-by: yucai <yyu1@ebay.com>
…st mem-leak. (ray-project#45752) Signed-off-by: yucai <yyu1@ebay.com>
…st mem-leak. (ray-project#45752) Signed-off-by: Richard Liu <ricliu@google.com>
Why are these changes needed?
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.