-
Notifications
You must be signed in to change notification settings - Fork 6k
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] Add config option to enable/disable TO vs FROM worker filter updates (aka filter syncing). #36204
[RLlib] Add config option to enable/disable TO vs FROM worker filter updates (aka filter syncing). #36204
Conversation
rllib/algorithms/algorithm_config.py
Outdated
@@ -1470,6 +1468,14 @@ def rollouts( | |||
enable_connectors: Use connector based environment runner, so that all | |||
preprocessing of obs and postprocessing of actions are done in agent | |||
and action connectors. | |||
use_worker_filter_stats: Whether to use the workers in the WorkerSet to | |||
update the central filters (held by the local worker). If False, stats | |||
from the workers will not be used ad discarded. |
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.
typo!
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.
fixed
rllib/utils/filter_manager.py
Outdated
for k in local_filters: | ||
local_filters[k].apply_changes(rf[k], with_buffer=False) | ||
|
||
# Should we update the remote workers' filters from the (now synched) |
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.
"now possibly synched?"
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.
fixed
Thanks for your review @ArturNiederfahrenhorst ! :) Will fix the issues and then merge. |
…config_option_to_not_include_eval_data_in_filter_stats
…updates (aka filter syncing). (ray-project#36204) Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Add config option to enable/disable TO vs FROM worker filter updates (aka filter syncing):
config.synchronize_filter
) to NOT update a WorkerSet's remote worker filters from the central filters (held by the local workers).This PR introduces the missing setting (
config.use_worker_filter_stats
) and clarifies the difference between these two options by better config property naming. The old setting name (config.synchronize_filter
) has been soft-deprecated and replaced byconfig.update_worker_filter_stats
.Why are these changes needed?
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.