edbeeching / godot_rl_agents

An Open Source package that allows video game creators, AI researchers and hobbyists the opportunity to learn complex behaviors for their Non Player Characters or agents
MIT License
942 stars 69 forks source link

Fix rllib #140

Closed edbeeching closed 1 year ago

edbeeching commented 1 year ago

Should resolve #139

Needs install from new venv with pip install godot-rl[rllib]

visuallization commented 1 year ago

@edbeeching haha oh noes! I was working on the same thing as you :D

https://github.com/edbeeching/godot_rl_agents/pull/141

edbeeching commented 1 year ago

@edbeeching haha oh noes! I was working on the same thing as you :D

141

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.

visuallization commented 1 year ago

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.

image

How is it for you? Maybe it is just realted of my godot project which uses an older version of the plugin.

visuallization commented 1 year ago

@edbeeching haha oh noes! I was working on the same thing as you :D

141

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.

Agreed! next time, the one who wants to tackle the problem, should just claim so in the issue.

edbeeching commented 1 year ago

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.

image

How is it for you? Maybe it is just realted of my godot project which uses an older version of the plugin.

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.

edbeeching commented 1 year ago

@visuallization would you mind installing from scratch with this PR + rllib? I want to be sure that we need to set the numpy version.

visuallization commented 1 year ago

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. image How is it for you? Maybe it is just realted of my godot project which uses an older version of the plugin.

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.

I am also on Linux now - so that's not it. It might be related to godotrl plugin - i will try upgrding the plugin and check again

visuallization commented 1 year ago

@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? pip show numpy

edbeeching commented 1 year ago

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? pip show numpy

Name: numpy
Version: 1.25.1
Summary: Fundamental package for array computing in Python
Home-page: https://www.numpy.org
Author: Travis E. Oliphant et al.
Author-email: 
License: BSD-3-Clause
Location: /home/edward/play/godot/godot_rl/godot_rl_agents/venv/lib/python3.10/site-packages
Requires: 
Required-by: contourpy, godot-rl, gym, Gymnasium, huggingface-sb3, imageio, matplotlib, onnx, onnxruntime, pandas, pyarrow, PyWavelets, ray, scikit-image, scipy, stable-baselines3, tensorboard, tensorboardX, tifffile

python version 3.10.9

visuallization commented 1 year ago

hmm I have python 3.9.16

visuallization commented 1 year ago

Hmm maybe it is related on how my actions look. 2 continuos and 1 discrete. my env is very similar to jumperhard. Does jumperhard work for you with latest numpy version?

visuallization commented 1 year ago

@edbeeching Okay so JumperHard does also not working for me with the current numpy version. Which environment are you using for testing? Is it maybe related to envs which are mixing continuous and discrete spaces?

visuallization commented 1 year ago

@edbeeching Okay so yes the actions are exaclty the problem, because the array consists of different types and sizes. see;

Reproducing in plain python: image

This is how the action look for jumperhard, whcih fails with latest numpy version: image

visuallization commented 1 year ago

@edbeeching fixed: https://github.com/edbeeching/godot_rl_agents/pull/140/commits/20c4df7ed73e04eeb04c0e4d16d00091dad7818d

Ivan-267 commented 1 year ago

@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 '1.21.2' on a local python 3.9 installation. I guess they later changed the warning to an error.

visuallization commented 1 year ago

@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.

image

visuallization commented 1 year ago

@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.

image

Ah okay it is because with your implementation we have to set --speedup explicitly via terminal

visuallization commented 1 year ago

Also fixed some issues with cleanrl and hp tuning

visuallization commented 1 year ago

@edbeeching we should probably make a new release for 0.6.1 as it fixes quite a few essential things.

edbeeching commented 1 year ago

@edbeeching we should probably make a new release for 0.6.1 as it fixes quite a few essential things.

Yes I agree, I am just writing a test for rllib training. I will look at #117 then we can look to merge both and release v0.6.1.

Ivan-267 commented 1 year ago

Would we have to have sb3 as an extra as well so that it doesn't get installed together with rllib? That might not be an optimal solution, just wondering about the error from the test.

edbeeching commented 1 year ago

@Ivan-267 yes I am trying to avoid that as I would like new users to pip install godot-rl and have sb3 installed out of the box. I am looking for a workaround / fix the tests.

edbeeching commented 1 year ago

ok while I am glad I made this test. Getting all these dependencies to work is going to be a headache.

edbeeching commented 1 year ago

ok a few options:

    • we remove sb3 as a dependency from the base install and require install with pip install godot-rl[sb3]
    • we have a more convoluted install process pip install godot-rl, pip uninstall sb3, pip install ray[rllib]

I actually prefer the third option for now, as it will mean we can test correctly and new users have sb3 which is the best entrypoint to the lib. Advanced users can follow the steps above for rllib support. What do you think @visuallization ?

Ivan-267 commented 1 year ago

Actually even with those changes I got:

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
godot-rl 0.6.1 requires stable-baselines3, which is not installed.

That said it did install rllib and start training in my cart demos env, but I don't know how the test would treat the error above.

There were some warnings, maybe related to my env (uses discrete actions only):

envs\test\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

envs\test\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.

envs\test\lib\site-packages\gymnasium\spaces\box.py:227: UserWarning: WARN: Casting input x to numpy array. [repeated 4x across cluster]

I'll add the change just to see if tests show the same error, I don't have a local setup to run the test install script in a virtual environment atm.

visuallization commented 1 year ago

ok a few options:

1. * we remove sb3 as a dependency from the base install and require install with `pip install godot-rl[sb3]`

2. * we wait for [[RLlib] Upgrade the gymnasium dependency to version the latest version (gymnasium==0.28.1) ray-project/ray#37393](https://github.com/ray-project/ray/issues/37393) before adding this fix

3. * we have a more convoluted install process `pip install godot-rl`, `pip uninstall sb3`, `pip install ray[rllib]`

I actually prefer the third option for now, as it will mean we can test correctly and new users have sb3 which is the best entrypoint to the lib. Advanced users can follow the steps above for rllib support. What do you think @visuallization ?

You mean for testing? I think the 3rd option is fine. I would love option number 2 but it might still take some time until it gets released (if it ever does) - so let's go for now with option 3, as I think it it is important to fix the current issues and we can still go with option 2, once ray releases the proposed upgrade. I also don't like option 1 too much because of the points you mentioned - it should be a easy as possible to get started with godotrl-agents.

On another note: I was wondering if we should fix our dependency versions? With that we can make sure that things don't randomly break if somebody installs the lib at a later point. We would then need to have a dedicated updated process, where we go through the dependencies every now and then, test things, and fix a new version. I think it might be actually worth it in terms of stabiility and future proof. @edbeeching @Ivan-267 what do you guys think?

visuallization commented 1 year ago

@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 :shrug: https://github.com/freqtrade/freqtrade/issues/8376

Ivan-267 commented 1 year ago

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 pip install .[rllib]

https://github.com/edbeeching/godot_rl_agents/actions/runs/5690825628/job/15424893195

Successfully installed PyWavelets-1.4.1 aiosignal-1.3.1 attrs-23.1.0 click-8.1.6 dm-tree-0.1.8 frozenlist-1.4.0 godot-rl-0.6.1 gym-0.21.0 gymnasium-0.26.3 gymnasium-notices-0.0.1 imageio-2.31.1 importlib-metadata-4.13.0 jsonschema-4.18.4 jsonschema-specifications-2023.7.1 lazy_loader-0.3 lz4-4.3.2 markdown-it-py-3.0.0 mdurl-0.1.2 msgpack-1.0.5 pyarrow-12.0.1 pygments-2.15.1 ray-2.6.1 referencing-0.30.0 rich-13.4.2 rpds-py-0.9.2 scikit-image-0.21.0 scipy-1.11.1 stable-baselines3-1.8.0 tensorboardX-2.6.1 tifffile-2023.7.18 typer-0.9.0

Locally I get this error with Conda and Python 3.9 or 3.10:

Collecting stable-baselines3 (from godot-rl==0.6.1)
  Using cached stable_baselines3-1.8.0-py3-none-any.whl (174 kB)
Collecting gym==0.21 (from stable-baselines3->godot-rl==0.6.1)
  Using cached gym-0.21.0.tar.gz (1.5 MB)
  Preparing metadata (setup.py) ... error
  error: subprocess-exited-with-error

  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [1 lines of output]
      error in gym setup command: 'extras_require' must be a dictionary whose values are strings or lists of strings containing valid project/version requirement specifiers.
      [end of output]

And if I try

python -m pip install --upgrade pip wheel==0.38.4

It just says:

Requirement already satisfied: pip in miniconda3\envs\rllib\lib\site-packages (23.2.1)
Requirement already satisfied: wheel==0.38.4 in miniconda3\envs\rllib\lib\site-packages (0.38.4)

It will install with the solution here: https://github.com/openai/gym/issues/3176#issuecomment-1560026649

I had to use:

python.exe -m pip install setuptools==65.5.0 pip==21

Before pip install .[rllib]

That does make the install process a bit complicated for rllib, but it may be possible to just use pip install ray[rllib] instead for now at least on local PC to simplify that step (there's no need to reinstall the older version of sb3).

When I run the examples install script and then try: pytest .\tests\test_rllib.py I get this error:

(RolloutWorker pid=3160) OSError: [WinError 216] This version of %1 is not compatible with the version of Windows you're running. Check your computer's system information and then contact the software publisher [repeated 9x across cluster]

I wasn't able to start the binaries either locally, even with trying gdrl.env_from_hub -r edbeeching/godot_rl_JumperHard the game exe seems incompatible with 64 bit windows.

Here is the entire content of JumperHard.exe after the download:

version https://git-lfs.github.com/spec/v1
oid sha256:784e2e3b2022a9421df07c764fc394ff0576147b16d186b9ed17fb725e96d4d4
size 68805120

After rebuilding the example in Godot, the test_rllib.py test does pass locally (quickly) but there are multiple warnings:

================================================================================================ 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 =========================================================================================
visuallization commented 1 year ago

The last test yesterday also did start successfully but was taking too long

@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. :)

Ivan-267 commented 1 year ago

The last test yesterday also did start successfully but was taking too long

@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): https://github.com/edbeeching/godot_rl_agents/actions/runs/5685626539/job/15422792046

godot-rl 0.6.1 requires stable-baselines3, which is not installed.
Successfully installed PyWavelets-1.4.1 aiosignal-1.3.1 attrs-23.1.0 click-8.1.6 dm-tree-0.1.8 frozenlist-1.4.0 gymnasium-0.26.3 gymnasium-notices-0.0.1 imageio-2.31.1 jsonschema-4.18.4 jsonschema-specifications-2023.7.1 lazy_loader-0.3 lz4-4.3.2 markdown-it-py-3.0.0 mdurl-0.1.2 msgpack-1.0.5 pyarrow-12.0.1 pygments-2.15.1 ray-2.6.1 referencing-0.30.0 rich-13.4.2 rpds-py-0.9.2 scikit-image-0.21.0 scipy-1.11.1 tensorboardX-2.6.1 tifffile-2023.7.18 typer-0.9.0

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 pytest .\tests\test_rllib.py on my local system (after rebuilding the exe).

Ivan-267 commented 1 year ago

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:

godot_rl/wrappers/sample_factory_wrapper.py:100: AttributeError
=========================== short test summary info ============================
FAILED tests/test_sample_factory.py::test_sample_factory_training - AttributeError: 'Namespace' object has no attribute 'seed'
edbeeching commented 1 year ago

Hey thanks to you both for continuing to work on this. I am travelling this weekend and will pick up on Monday.

visuallization commented 1 year ago

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:

godot_rl/wrappers/sample_factory_wrapper.py:100: AttributeError
=========================== short test summary info ============================
FAILED tests/test_sample_factory.py::test_sample_factory_training - AttributeError: 'Namespace' object has no attribute 'seed'

@Ivan-267 that's awesome! Thanks for fixing the tests including the seeding issue - I forgot about the sample factory example env!

edbeeching commented 1 year ago

@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.

visuallization commented 1 year ago

@edbeeching @Ivan-267 Ray is updating gymnasium: https://github.com/ray-project/ray/issues/37393#issuecomment-1660369444. :tada: Will be released in around 3 weeks. So we could keep an all option and fix the version for it, so it stays compatible in the future.

edbeeching commented 1 year ago

@visuallization , this is great. I will merge as is for now as there are a number of other fixes in this PR.