-
Notifications
You must be signed in to change notification settings - Fork 13
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
Added usage section in ReadMe and basic example for simulator #314
Added usage section in ReadMe and basic example for simulator #314
Conversation
When loading the icub model, I encountered in the following warning:
I am having issues in backtracking the source, @flferretti do you perhaps know which part of rod raises this warning ? thanks ! |
Hi @CarlottaSartore, thanks for the contribution! The warning is not raised from |
Related: scipy/scipy#19512 and https://mail.python.org/archives/list/[email protected]/message/LXT4ZGNP4OVAARSKEOM5AGQCANH6ZV5K/ . I totally agree that this should not be a warning, or there should be a way to provide this conversion without triggering a warning, but unfortunately there was not a follow up in the mailing list. |
Early comment: can we use - import icub_models
- model_path = icub_models.get_model_file("iCubGazeboV2_5")
+ import robot_description.icub_description
+ model_path = robot_description.icub_description.URDF_PATH |
I strongly prefer use any other robot in that case. There is nothing ensuring that the |
I see, thanks for the explanation! I was wondering though if we need the actual IIT model since this is just an example |
The problem is that lab users starts from examples, that they copy and they start using assuming that that is the "best way" to do, especially if the info come from an official repo of the lab. I really do not look forward to having to explain each new student in the lab why they should not be using |
(I also edited my previous comments for more details). |
I opted to use the icub models since it is the model of a humanoid robots, and I wanted to use a robot since this is the main goal of the simulator, and the icub is the model I am the more confident with. |
If it helps reach consensus, feel free to use
The tricky aspect there is that you also need to download the meshes if you want the visualizer to work. |
Thanks @traversaro, I understood the problem. It seems to me that the available options are:
|
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.
Thanks a lot for working on this @CarlottaSartore! I added some comments
Stickbot is nice as it is a self-contained URDF. I think there is also some value in showing to users how to load a physical URDF. My assumption is that most users will want to use the simulator with their own model for which they either have the absolute file path or the relocatable |
Concerning the usage of model descriptor, I would stay with the icub models dependency. even though it introduces a new dependencies it shows how:
|
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.
Thanks @CarlottaSartore ! It LGTM! I will update the title of the examples/jaxsim_as_physics_engine_advanced.ipynb
notebook since up to now it's the same of the non advanced one :)
Thanks @xela-95 you're right! I have also updated the example readme with the new example ! |
now the tests are passing! Can we merge this PR or is there any other fbk ? @xela-95 @flferretti |
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.
Thanks a lot @CarlottaSartore! I added some final minor comments and then we can merge for me
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Alessandro Croci <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
Co-authored-by: Filippo Luca Ferretti <[email protected]>
1e48c6d
to
9a7d125
Compare
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.
Thanks @CarlottaSartore ! LGTM 🚀
branch rebased, merging ! |
This PR:
example_physics_engine
inexample_physics_engine_advanced
📚 Documentation preview 📚: https://jaxsim--314.org.readthedocs.build//314/