Fix ray_rllib solver after the upgrade to ray 2.7.0 #284
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
b099f6f to
7df3abc
Compare
neo-alex
approved these changes
Nov 30, 2023
| def test_as_rllib_env_with_autocast_from_singleagent_to_multiagents(): | ||
| ENV_NAME = "CartPole-v1" | ||
|
|
||
| domainupcasted = GymDomain(gym.make(ENV_NAME)) |
There was a problem hiding this comment.
As in the previous PR, I suggest renaming it upcast_domain
There was a problem hiding this comment.
of course, i change it right away
| assert RayRLlib.check_domain(domain) | ||
|
|
||
| # solver factory | ||
| config_factory = lambda: PPO.get_default_config().resources( |
There was a problem hiding this comment.
A config_factory seems overkill here (why not just config?)
There was a problem hiding this comment.
because it is crashing later when reusing it with the loaded solver (line 89). When loading, we reinitialize the underlyning algorithm and it does not want to use the previous config which has been "frozen" during training. So we need a fresh new config at that second step which is testing save/load.
And to be sure having the same config values as before I used this factory (perhaps a bit overkill, but i do not write twice the same values when i can avoid it since sooner or later, it introduce bugs when editing it only in one place...)
There was a problem hiding this comment.
I added a comment in the code to explain it.
There was a problem hiding this comment.
Oh OK, and then I suppose that the config being frozen is an issue (even if we don't modify it?)...
This solver class wraps ray.rllib algorithms.
Since we upgrade to gymnasium, the dependency in ray also upgrade from
1.10 to >=2.7.0 and the rllib api changed a bit.
Main changes/fixes:
- ray.rllib.agents becomes ray.rllib.algorithms, Trainer -> Algorithm
- policy_mapping_fn: maps now agent_id + episode + worker -> policy_id
- algo.compute_action -> compute_single_action()
- MultiAgentEnv follows now gym0.26/gymnasium convention => possibility
to use MultiAgentEnvCompatibility rllib wrapper. We choose here,
consistently with the work done in skdecide.hub.domain.gym to
keep the old class AsRLlibMultiAgentEnv with the gym 0.21 api and wraps it.
More precisely:
- the old env is renamed as AsLegacyRLlibMultiAgentEnv and derives from AsLegacyGymV21Env
in order to share some methods and define some missing ones (as render(), seed(), close())
- the new AsRLlibMultiAgentEnv derives from MultiAgentEnvCompatibility which itself
derives from MultiAgentEnv
- MultiAgentEnv needs a get_agent_ids() (or at least a _agent_ids
attribute)
- MultiAgentEnv needs to define observation_space and action_space
attributes. Preferably as gym.spaces.Dict in order to allow implicit
implementation of new methods action_space_sample() and observation_space_sample()
- done output in step() must be a dictionary with a key by agent + a
__all__ key
- algo.load() takes directly the directory path (which is overwritten at
each store)
Other fixes done here:
- __init__() of RayRLlib was using a mutable object as default value for
policy_config which can lead to unforeseen bugs
- main script moved as test in tests/ together with a test dedicated to
AsRLlibMultiAgentEnv
- fixes "__all__" value of done dictionary as MultiAgent domains are
supposed to returned a dictionary agent_id->boolean instead of a
single boolean
- update the examples/rllib_solver.py
- test also single agent domain to check the autocast works properly
with rllib
…class definition We faced serialization issues with ray.rllib solver and python 3.10. This seemed to be due to NewType definition inside Domain class. Actually the NewType assumed that the resulting is belonged by the module and not the class which causes pickle issues. See for instance https://stackoverflow.com/a/4677063 for similar issue.
The tests are hanging for ever on macos 11 in guthub actions with the
following message looping:
> (autoscaler +5h31m36s) Warning: The following resource request cannot be scheduled right now: {'CPU': 1.0}. This is likely due to all cluster resources being claimed by actors. Consider creating fewer actors or adding more nodes to this Ray cluster.
We limit the cpu quota allowed per worker to avoid this.
This was referenced Dec 1, 2023
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.
The
RayRLlibsolver class wrapsray.rllibalgorithms.Since we upgrade to gymnasium, the dependency in ray also upgrade from
1.10 to >=2.7.0 and the rllib api changed a bit.
Main changes/fixes:
ray.rllib.agentsbecomesray.rllib.algorithms,Trainer->Algorithmpolicy_mapping_fn: maps now agent_id + episode + worker -> policy_idalgo.compute_action->compute_single_action()MultiAgentEnvfollows now gym0.26/gymnasium convention => possibilityto use
MultiAgentEnvCompatibilityrllib wrapper. We choose here,consistently with the work done in
skdecide.hub.domain.gymtokeep the old class
AsRLlibMultiAgentEnvwith the gym 0.21 api and wraps it.More precisely:
AsLegacyRLlibMultiAgentEnvand derives fromAsLegacyGymV21Envin order to share some methods and define some missing ones (as
render(),seed(),close())AsRLlibMultiAgentEnvderives fromMultiAgentEnvCompatibilitywhich itselfderives from
MultiAgentEnvMultiAgentEnvneeds aget_agent_ids()(or at least a_agent_idsattribute)
MultiAgentEnvneeds to defineobservation_spaceandaction_spaceattributes. Preferably as
gym.spaces.Dictin order to allow implicitimplementation of new methods
action_space_sample()andobservation_space_sample()doneoutput instep()must be a dictionary with a key by agent + a"__all__ "keyalgo.load()takes directly the directory path (which is overwritten ateach store)
Other fixes done here:
__init__()ofRayRLlibwas using a mutable object as default value forpolicy_config which can lead to unforeseen bugs
AsRLlibMultiAgentEnv"__all__"value of done dictionary asMultiAgentdomains aresupposed to returned a dictionary agent_id->boolean instead of a
single boolean
with rllib