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

20.11.2023 results #12

Closed Chiniklas closed 9 months ago

Chiniklas commented 10 months ago
fwiebe commented 9 months ago

Hi @Chiniklas thanks for your contribution, I would like to ask you to make the following changes:

  1. The paths where you put the data for the lqr controller parameters are missing the model subfolder. I.e. it should be data/controller_parameters/design_A.0/model_1.1/pendubot/lqr/ and data/controller_parameters/design_C.1/model_1.1/acrobot/lqr/

    Note also that it is design_C.1 in the acrobot case. There are already LQR parameters for design_C.1/model_1.1 in the repository (acrobot and pendubot). If your parameters differ from them, let me know.

    I noticed that the model parameters for design_A.0 which you committed with the LQR parameters do not match any of the existing model parameters. Judging from the lengths (l1=0.2 and l2=0.26) these seem to be parameters for design_C. Do you wish to submit new model parameters? If so please do under the path "data/system_identification/identified_parameters/design_X.x/model_Y.y"

  2. There are now two sac policies at data/policies/design_C.0/model_3.0/acrobot/SAC/sac_model.zip and data/policies/design_C.1/model_1.0/acrobot/SAC_not_working_on_hardware/best_model.zip

    Do you want to have both in the repository? If yes I would propose to store them under the paths data/policies/design_C.0/model_3.0/acrobot/SAC/sac_model.zip and data/policies/design_C.1/model_1.0/acrobot/SAC/sac_model.zip

    You can add a readme file with the information that the policy did not work on the hardware.

  3. In "experiments/tmotors_saclqr.py" you load model parameters from "model_data/model_parameters.yml" but they are not committed. If you want to use model parameters which are not yet in the repository please add them under "data/system_identification/identified_parameters/..."

  4. Please remove the print statements in "src/python/double_pendulum/controller/SAC/SAC_controller.py"

Thanks again :)

Chiniklas commented 9 months ago

Changes have been made regarding each comment, please check.

fwiebe commented 9 months ago

Thanks, I merged the pull request.