Fixed package dependencies

[alhasacademy96]

Fixed a dependency issue with training suing stable-baselines3.

The PR simply adds the "shimmy" package which is required by stable-baselines3 when using gym.

[kozzy97]

This works for me @alhasacademy96 , I don't get any conflicts, but I am wondering whether we should specify shimmy here, as opposed to having a specific agents module/submodule (as @wschella suggests) and specifying shimmy/mlagents/etc. there? I won't approve until we have discussed this, just in case.

[alhasacademy96]

I don't understand your comment kozzy. We are already requiring mlagents as default to be imported as this is fundamental, but shimmy is not related to mlagents, but for external RL training environments. Having a submodule means creating another repository within: https://gist.github.com/gitaarik/8735255 Ben and Seraphina had the same problem as were using stable-baselines3, which requires shimmy for the API conversion when using pettingzoo or gymnasium. Therefore, having shimmy as a requirement is practical. https://github.com/Farama-Foundation/Shimmy.

I guess we could specify an optional dictionary such as:

extras_require = {
        'full': ['matplotlib', 'tensorflow', '**shimmy**'] (as an example)
    },

which the user can then import them as: 'pip install animalai[shimmy]'

[wschella]

I don't think Kozzy was referring to a Git module, but instead a separate Python package for all sorts of agents integration. Note: Working with extras_require would also work for this as an intermediary route.

Shimmy is light enough of a package that it could be a dependency without much issue, although it does depend on Gymnasium itself, which is enough of a problem, since ml-agents depends on Gym (without nasium).

The general thing is that it is bad practice to include a dependency your code does not depend on. The core animal-ai package is an interface over an ml-agents environment (potentially wrapped in an gym.Gym). Nothing depends on Shimmy.

If we would want to expose a Gymnasium.Gym interface directly (as opposed to Gym.Gym interface we have now), we could do that, but we don't need Shimmy for that. Gymnasium just does that [https://gymnasium.farama.org/api/wrappers/misc_wrappers/#gymnasium.wrappers.EnvCompatibility](out of the box).

If people want Shimmy or Stablebaselines, they just have to do pip install animalai shimmy stable-baselines3.

We are not the consumers of Shimmy, only some users are, we shouldn't force the Shimmy dependency on users who don't consume it.

That said, since Gym is deprecated, it would be good to move to the Gymnasium API, using their wrapper over the ml-agents Gym API so users don't need to do this wrapping themselves. That's a separate issue tho.

[alhasacademy96]

Yeah its just my point is two of the new comers have had problems with this. I think we will add the optional extras parameter so it is easier for people to get to work straight away. Thanks for the comments guys. I'll make the changes and enjoy my leave :)

[wschella]

We should fix the underlying issue (exposing an old Gym API instead of a Gymnasiun API), not use bad-practice bandaids.

This can be fixed in documentation as well, with a short AAI + Stablebaselines section.

[alhasacademy96]

If you're up for it wout go ahead :)

[alhasacademy96]

Actually it seems like the process is linear. I will look into this once i get back from my leave. My PR was just to be on the safe side and currently i have a lot of other things to do, but i think this is also very important...

[alhasacademy96]

However (i've reopened the PR), because this would entail a bit of work and i am currently occupied with other tasks, i would like this to be merged so that in case I don't work on researching and potentially confirming using gymnasium is a better alternative to gym before january (as gymnasium is literally a fork of gym and if i do make the change, then it should be quite simple to do so as most things would remain the same), any new user would not be prone to this dependency issue. Shimmy is very light and there really is no need to be discussing this as it will be an optional dependency and have very little impact overall, with clear instructions on when to use it and how to obtain it in the docs.

[kozzy97]

I must admit I do not know the best practices at all here. It seems @alhasacademy96 wants the optional package req to be added, and it works for now, so I'll approve in deference to your expertise. But it looks like updates to gymnasium and ml-agents 1 are required ASAP.

[wschella]

There indeed shouldn't be no need to discuss this, as this is not how things are done. We have no code depending on Shimmy, so it's not a dependency, also not an optional one. It's not enough, as you need to install Shimmy with optional dependencies to make it work with Gym, and it shouldn't be needed, as Gymnasium (the interface Stablebaselines implements) supports the conversion out of the box.

The impact this creates is that users will depend on this, and we have will have incompatibilities and confusion whenever we want to eventually clean this up.

In any case, I've created a PR #4 about how one can use Stablebaselines with AnimalAI.

Feel free to merge of course, but my strong opinion is we're moving the project in the wrong direction with this.

PS: ml-agents 1.0.0 still implements the old Gym API instead of Gymnasium. We can convert on our side if we want tho, which I would recommend. This would create incompatibilities with ml-agents Agents, but I doubt the script in the repo still works in any case...

[alhasacademy96]

I got an error which stated that shimmy is required whilst trying to use stable baselines3 during my tests. That's all. I'm just trying to make sure people with very little experience have very little bumps on their experience with animalai as it currently stands we are using gym and not gymnasium on our side. Please feel free to upgrade to gymnasium on our side as this should be relatively simple. Please also remember to update me on the progress should you wish to take this task. Thanks

P.s. most of the code for mlagents using gym would remain the same as gymnasium is a fork of gym. This would still require a lot of testing and making sure the code communicates as normal.

[wschella]

I understand, I have the same motivation. I just want to guard against introducing other bumps for different users then.

What version of Stablebaselines are Ben, Seraphina and you using? Because the latest is not compatible with ml-agents=0.30.0 due to different dependencies on torch.

- AnimalAI == 3.0.5
  requires Python >=3.6, <3.10
  requires ml-agents== 0.30.0
  ..requires "torch>=1.8.0,<=1.11.0;(platform_system!='Windows' and python_version>='3.9')",
  ..requires "torch>=1.6.0,<1.9.0;(platform_system!='Windows' and python_version<'3.9')",
- Stable Baselines3 == 2.2.1
  requires torch >= 1.13

[wschella]

This is the latest set I was able to install:

# - AnimalAI == 3.0.5
#   requires Python >=3.6, <3.10
#   requires ml-agents== 0.30.0
#   ..requires "torch>=1.8.0,<=1.11.0;(platform_system!='Windows' and python_version>='3.9')",
#   ..requires "torch>=1.6.0,<1.9.0;(platform_system!='Windows' and python_version<'3.9')",
#
# - Stable Baselines3 == 2.0.0
#   requires gymnasium==0.28.1
#   requires torch>=1.11
#   Note: Later version of Stable Baselines require "torch>=1.13", which is not compatible with ml-agents.
dependencies = [
    "animalai==3.0.5",
    "gymnasium>=0.27.0",
    "stable-baselines3<2.1.0",
]
requires-python = ">= 3.9"

It's a pity that ml-agents depends on torch, it creates a big mess (like this), and is a good reason we should not make it worse by depending on extra stuff.

[wschella]

I've created a template repository using for integrating Stablebaselines with AnimalAI: https://github.com/Kinds-of-Intelligence-CFI/animal-ai-stablebaselines3

It seems Shimmy is needed, as Gymnasiums EnvCompatibility will be deprecated, and in general, Stablebaseline complains. This still doesn't mean we should specify it as a dependency here (as it is more likely to cause trouble). We can point there to reduce hurdles for new users. I'll update #4 correspondingly.

[alhasacademy96]

ATTENTION!

I've paused this PR in the meantime. We will come back to it in January when I will have time to carry out some tests.