-
Notifications
You must be signed in to change notification settings - Fork 71
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
Fix rllib #140
Fix rllib #140
Conversation
@edbeeching haha oh noes! I was working on the same thing as you :D |
Doh! Luckily it is not a big change. We should be a bit more organized with assigning things in the future. It seems we made some complementary changes, if you want to pick up the ones from my branch and add them to yours? I will then close mine and we can get yours merged. |
I think yours is nicer (no check if obs exists and making infos a list), so we should go with yours. The only thing I noticed is that we apparently have to keep the fixed numpy==1.23.5 version. At least for me it throws an error if we don't keep it. How is it for you? Maybe it is just realted of my godot project which uses an older version of the plugin. |
Agreed! next time, the one who wants to tackle the problem, should just claim so in the issue. |
It is strange I don't see to have problem with numpy, but I am running on Linux so perhaps that is it? I will add your docs changes to my PR. |
@visuallization would you mind installing from scratch with this PR + rllib? I want to be sure that we need to set the numpy version. |
I did just that and it is not working for me :(. Not even after using the godot_rl addon from main. which addon version are you using? And what is the numpy version you are using? |
python version 3.10.9 |
hmm I have python 3.9.16 |
Hmm maybe it is related on how my actions look. |
@edbeeching Okay so JumperHard does also not working for me with the current numpy version. |
@edbeeching Okay so yes the actions are exaclty the problem, because the array consists of different types and sizes. see; This is how the action look for jumperhard, whcih fails with latest numpy version: |
@edbeeching fixed: 20c4df7 |
Nice catch, this is what I get when trying something similar: np.array([1,[2,3]])
<stdin>:1: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray with numpy |
@edbeeching Your implementation does not seem to take num_workers correctly into account. Check this out: Both versions where run with the same config but your implementation took around 20m while mine took around 10m. Not sure what's going on. Rerunning the experiment to make sure I did not mix anything up. |
Ah okay it is because with your implementation we have to set --speedup explicitly via terminal |
…e if it reolves the issue
@edbeeching I fixed the install step in the tests but now the actual test seems to timeout for rllib. Check this regarding fixing the wheel version 🤷 freqtrade/freqtrade#8376 |
About fixing versions, I also think that it would help with stability, but then we may have to make sure that the version (or version ranges) specified are compatible for most operating systems / Python versions that we want to support. Maybe that's the case for all or most packages, just something to consider. The last test yesterday also did start successfully but was taking too long, although now it should install whichever rllib version is set in the requirements. Edit: I noticed that now in the test, sb3 1.8 is installed when using https://github.com/edbeeching/godot_rl_agents/actions/runs/5690825628/job/15424893195
Locally I get this error with Conda and Python 3.9 or 3.10:
And if I try
It just says:
It will install with the solution here: I had to use:
Before That does make the install process a bit complicated for rllib, but it may be possible to just use When I run the examples install script and then try:
I wasn't able to start the binaries either locally, even with trying Here is the entire content of
After rebuilding the example in Godot, the ================================================================================================ warnings summary ================================================================================================
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\logger\tensorboardx.py:41: DeprecationWarning: `np.bool8` is a deprecated alias for `np.bool_`. (Deprecated NumPy 1.24)
VALID_NP_HPARAMS = (np.bool8, np.float32, np.float64, np.int32, np.int64)
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\logger\tensorboardx.py:169: DeprecationWarning: `np.bool8` is a deprecated alias for `np.bool_`. (Deprecated NumPy 1.24)
VALID_NP_HPARAMS = (np.bool8, np.float32, np.float64, np.int32, np.int64)
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gym\envs\registration.py:250: DeprecationWarning: SelectableGroups dict interface is deprecated. Use select.
for plugin in metadata.entry_points().get(entry_point, []):
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\torch\utils\tensorboard\__init__.py:4: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
if not hasattr(tensorboard, "__version__") or LooseVersion(
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\torch\utils\tensorboard\__init__.py:6: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
) < LooseVersion("1.15"):
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\tune.py:258: UserWarning: Passing a `local_dir` is deprecated and will be removed in the future. Pass `storage_path` instead or set the `RAY_AIR_LOCAL_CACHE_DIR` environment variable instead.
warnings.warn(
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\tune.py:735: DeprecationWarning: checkpoint_freq is deprecated and will be removed. use checkpoint_config.checkpoint_frequency instead.
warnings.warn(
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\tune.py:742: DeprecationWarning: checkpoint_at_end is deprecated and will be removed. use checkpoint_config.checkpoint_at_end instead.
warnings.warn(
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gymnasium\spaces\box.py:127: UserWarning: WARN: Box bound precision lowered by casting to float32
logger.warn(f"Box bound precision lowered by casting to {self.dtype}")
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gymnasium\utils\passive_env_checker.py:141: UserWarning: WARN: The obs returned by the `reset()` method was expecting numpy array dtype to be float32, actual type: float64
logger.warn(
tests/test_rllib.py::test_rllib_training
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gymnasium\utils\passive_env_checker.py:165: UserWarning: WARN: The obs returned by the `reset()` method is not within the observation space.
logger.warn(f"{pre} is not within the observation space.")
tests/test_rllib.py: 73 warnings
C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\tensorboardX\summary.py:153: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)
scalar = float(scalar)
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================================================== 1 passed, 84 warnings in 46.77s ========================================================================================= |
@Ivan-267 Oh really? Maybe I misslooked but it seemed like it would still throw the same pip install error Ed reported. Anyways if your version works, please commit. :) |
I accidentally made it as two commits (from two single comment suggestions), but the second one did pass that part (not everything): It just shows the same error message as I got locally (but it doesn't halt the test):
However I wouldn't say it works because the test is still taking too long as with your case. I'm not sure why, we could try changing some config and see if it makes a difference. I was trying the test itself locally with pytest, but I found an issue with the example download step (for some reason it's not cloning the large files, e.g. the exe for me). However my local Windows environment may not be exactly the same as the test environment (or maybe I missed some step), as I see the test env downloads the examples just fine. The passed test with warnings above was from |
Just checking if it makes a difference
It looks like it is passing the rllib test now (similarly to as on my PC, with warnings). I'm not sure if it was the timesteps, changing num workers to 1 (maybe the multiple workers cause some issue with test env? it works OK on my local setup), or some of the other changes that fixed it. You could check the remaining error:
|
Hey thanks to you both for continuing to work on this. I am travelling this weekend and will pick up on Monday. |
@Ivan-267 that's awesome! Thanks for fixing the tests including the seeding issue - I forgot about the sample factory example env! |
@visuallization or @Ivan-267 thanks for fixing the tests. I just pushed an update to the rllib installation instructions. If you can approve the PR then I will merge. |
fix typo
@edbeeching @Ivan-267 Ray is updating gymnasium: ray-project/ray#37393 (comment). 🎉 Will be released in around 3 weeks. So we could keep an |
@visuallization , this is great. I will merge as is for now as there are a number of other fixes in this PR. |
Should resolve #139
Needs install from new venv with
pip install godot-rl[rllib]