aidudezzz / deepworlds

Examples and use cases using the deepbots framework (https://github.com/aidudezzz/deepbots) with the Webots robot simulator.
GNU General Public License v3.0
50 stars 23 forks source link

KHR-3HV - fix coordinate system #76

Closed KelvinYang0320 closed 1 year ago

KelvinYang0320 commented 2 years ago

Resubmit KHR-3HV in https://github.com/aidudezzz/deepworlds/pull/49 with FLU/ENU and refactoring.

KelvinYang0320 commented 2 years ago

@tsampazk Should I move KHR-3HV readme.md to deepbots-tutorials?

tsampazk commented 2 years ago

@tsampazk Should I move KHR-3HV readme.md to deepbots-tutorials?

Yes, @KelvinYang0320, you can move it there and i'll fix some of its issues on the tutorials repo

KelvinYang0320 commented 2 years ago

@tsampazk Since we have moved KHR-3HV tutorial to deepbots-tutorials, should I remove it and create another simple and short README?

tsampazk commented 2 years ago

Yes @KelvinYang0320 that sounds great.

tsampazk commented 2 years ago

Along with that, you can restructure the directory to be similar to other example directories in deepworlds, instead of the tutorials directory structure

KelvinYang0320 commented 2 years ago

@tsampazk Hi, there! There are still some warning messages when running this example. SB3: warning Ray: warning2

Also, I am not sure if it converges well since it does not converge in my experiments and this reward-episode plot looks not good enough. There might be some problem with these lines. self.motorPositionArr[i] += ac is not synchronized with self.motorList[i].setPosition(ac).

I think we can merge this PR first and open another issue/PR to address these problems since this PR is mainly for conversion to FLU/ENU.

KelvinYang0320 commented 2 years ago

@tsampazk Thank you for the detailed review. I update the to-do list and convert this PR to draft.

KelvinYang0320 commented 1 year ago

@tsampazk I have resolved all conversations and opened a new training issue for this example so that we can focus on the coordinate system and coding style in this PR.

KelvinYang0320 commented 1 year ago

@tsampazk There are still a few warning messages:

tsampazk commented 1 year ago

@tsampazk There are still a few warning messages: ... Should we fix them in this PR?

If you think that they can be fixed with not a lot of effort then feel free to fix them, otherwise i feel that it is not critical to get this merged. Most glaring issues in the example have been fixed!

KelvinYang0320 commented 1 year ago

If you think that they can be fixed with not a lot of effort then feel free to fix them, otherwise i feel that it is not critical to get this merged. Most glaring issues in the example have been fixed!

@tsampazk I think it will take a while to fix them, so I agree with you! Should I squash all commits to one commit?

tsampazk commented 1 year ago

Should I squash all commits to one commit?

Yes, also there seems to be some conflicts in the wbt file that need to be resolved, then we can go ahead and merge this!

KelvinYang0320 commented 1 year ago

@tsampazk Does the rebasing look fine?:smile: