Skip to content
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

Separate the NeuroMechFly class into an environment, fly class and potential other classes #120

Closed
stimpfli opened this issue Feb 13, 2024 · 2 comments
Assignees

Comments

@stimpfli
Copy link
Contributor

Refactor the code keeping in mind that the environment could accommodate:

  • More flies
  • Other features than an arene (e.g ball, ...)
@sibocw
Copy link
Contributor

sibocw commented Feb 15, 2024

As we discussed today:

  • Let's have a Simulation class which is the main MDP environment (ie. implements gym.Env)
  • Simulation contains an Arena object and one or more NeuroMechFly instances
  • The dm_control.mjcf.physics.Physics instance will belong to Simulation instead of NeuroMechFly

The advantages are:

  • If we need to modify the arena (eg. change the object position in an arena with a moving object), we don't need to do it strangely by accessing the arena through the fly.
  • A cleaner interface for simulations with multiple flies (useful for social behaviors).

Questions:

  • Do the NeuroMechFly and Arena need an inverse link to the Simulation that they belong to? I don't like this because it makes the UML diagram messy and recurrent, but it might be necessary if we want to have an interface for the user to do things with the arena or the flies that would require access to the Physics instance.

@tkclam: Look at Barbara and Florian's semester project, they've done something like this already.

I don't think this is urgent but I do think it's better to do this now than later, as this is a major API change. If maintaining backward compatibility is too complicated, we can also forget about it and make a non-backward-compatible version as the paper is updated for revision. I think it's a reasonable thing to do.

@tkclam tkclam self-assigned this Feb 15, 2024
@sibocw
Copy link
Contributor

sibocw commented Apr 5, 2024

This is being implemented in https://github.com/NeLy-EPFL/flygym/tree/new-api

@sibocw sibocw closed this as completed Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants