dfki-ric-underactuated-lab / double_pendulum

Dual purpose Acrobot and Pendubot Platform
BSD 3-Clause "New" or "Revised" License
33 stars 22 forks source link

pull request for simulation phase of the RealAIGym competition #4

Closed Chiniklas closed 1 year ago

Chiniklas commented 1 year ago

Hello,

A new policy-based controller is submitted, which is SAC controller based on Stable Baseline3 library.

Chi Zhang

fwiebe commented 1 year ago

Hello @Chiniklas ,

Thank you for your pull request! I have the following remarks and questions regarding your submission:

  1. .idea and .vscode Are these folders needed for something in this repository? If not please delete them from the pull request.

  2. Dockerfile Please add the python dependency shimmy to the src/python/setup.py file. Please change the line

    RUN git clone https://github.com/Chiniklas/double_pendulum.git

    to the main repository instead of your fork. Please comment the line

    RUN git checkout v0.1.0
  3. policy files Consider placing the policy files, i.e.

    examples/reinforcement_learning/SAC/best_model/acrobot_model.zip
    examples/reinforcement_learning/SAC/best_model/pendubot_model.zip
    leaderboard/robustness/acrobot/SAC/sac_model.zip
    leaderboard/robustness/pendubot/SAC/sac_model.zip
    leaderboard/simulation/acrobot/SAC/sac_model.zip
    leaderboard/simulation/pendubot/SAC/sac_model.zip
    src/python/double_pendulum/controller/SAC/sac_training/best_model/best_model.zip

    in the main data folder. I would suggest the structure

    data/policies/<design_X.X>/<model_X.X>/<robot>/SAC/<your_policy>

    similar to the stored trajectories in the same data folder. For the naming of design and model see here. <robot> would be either 'acrobot' or 'pendubot'. If you have multiple policies for the same design, model, robot and method you can create subfolders on the last level.

  4. examples/reinforcement_learning/SAC/check_state.py The script returns an error because the load_path =data/pendubot/lqr/roa/rho in line 5 points to the folder data instead of lqr_data.

  5. examples/reinforcement_learning/SAC/sac_training I think, this folder contains you training attempts. These do not need to be in this examples folder. Please remove them from the pull request.

  6. experiments/tmotors_donothing.py You changed a few minor things in this experiment file which are not related to your controller. Please undo these changes.

  7. leaderboard controller files The SACController class is redefined in every con_sac_lqr.py file. Is there a reason for this or could the controller also be imported from src/python/double_pendulum/controller/SAC/SAC_controller.py?

  8. leaderboard data The leaderboard data does not need to be commited. The leaderboard results will be computed with a dedicated workflow and stored in a separate repository dedicated to the leaderboard. Please remove the result files

    leaderboard/robustness/acrobot/data/*
    leaderboard/simulation/acrobot/data/*
    leaderboard/robustness/pendubot/data/*
    leaderboard/simulation/pendubot/data/*
    leaderboard/simulation/pendubot/data_loc/*

    from the pull request. I am sorry, that was probably not so clear in the competition/repository rules.

  9. leaderboard/robustness/pendubot/plot.py Do you want this plotting script to be here? Otherwise, I would propose to remove it.

  10. leaderboard/simulation/acrobot and leaderboard/simulation/pendubot Do you want the files

    leaderboard/simulation/acrobot/create_urdf.py
    leaderboard/simulation/acrobot/dev_con_ilqrfree_lqr.py
    leaderboard/simulation/acrobot/timetest.py
    leaderboard/simulation/pendubot/create_urdf.py

    here? Otherwise, I would propose to remove them.

  11. src/python/double_pendulum The src folder is intended only for the reusable classes and functions which will be installed with this python package. In src/python/double_pendulum/controller/SAC there is a plots and a sac_training folder containing data of your training, I assume. Please either remove these folders or if they are needed move the data somewhere else in the repository, such as the data folder in the root directory. If there is data you want to submit and you are unsure where to put it feel free to contact me.

Thanks again for submitting a cool controller!

fwiebe commented 1 year ago

@Chiniklas Thank you for addressing the remarks, I merged the PR into the main repository