-
Notifications
You must be signed in to change notification settings - Fork 10
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
Cleaning start #6
Conversation
From now on, we stop using STLIB & AVEC from here. sofagym (lib) & examples (env) are separated.
env is envs now to match with gym lib we could have error o that since some paths are hard
sofagym/rpc_server.py
Outdated
@@ -525,7 +525,7 @@ def start_scene(config, nb_actions): | |||
# Run the first client | |||
def deferredStart(): | |||
sdict = str(config) | |||
subprocess.run([config['python_version'], path+"common/rpc_client.py", sdict, str(nb_actions), str(port_rpc)], | |||
subprocess.run([config['python_version'], path+"sofagym/rpc_client.py", sdict, str(nb_actions), str(port_rpc)], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion for another PR: modifier config["python_version"] par sys.executable pour sortir la version de python utilisé de la config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are AVEC & STLIB used anywhere in the project? In an example?
If yes, documentation should be updated to explain how to use AVEC and STLIB as an external dependency. Is it your plan when you say "to do -> add requirements"?
exactly |
STLIB was added to the project a long time ago when the master STLIB was not up to date with SofaPython3, it is probably not needed anymore. |
Yes, AVEC was just to show that we could use any kind of RL algorithms, even the ones that are coming from the actual research. For the library it is not very important I think... |
I started the readme update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in requierements.txt -> requirements.txt (to update in readme too)
* [BeamAdapter](https://github.com/sofa-framework/BeamAdapter) (fetchable within sofa) | ||
* [SPLIB](https://github.com/SofaDefrost/SPLIB) | ||
* [STLIB](https://github.com/SofaDefrost/STLIB) | ||
* [SoftRobots](https://github.com/SofaDefrost/SoftRobots) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why BeamAdapter and SoftRobots are not optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are used in almost all of the envs examples. However, actually, they are not really mandatory. I will check that and correct accordingly.
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
Refactoring the repo.
AVEC & STLIB lib are gone. They don't have there place here.
to do -> add requirements
sofagym folder contains the sofagym lib.
We can find examples scene in env (was envs before but rename to env to imitate the gym lib).
to do -> i think it's strange how it is done right now. To me it's should be a repo root.
to do -> make corrections everywhere to be sync with sofa 22.06. Some calls are deprecated...