Attempt to update to modern PyMunk #36
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @iandanforth, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request addresses an issue where a PyMunk update caused coordinate system changes, breaking the gym example. The changes include adjusting gravity direction, arm orientation, and PID control parameters to align with the new PyMunk version (6.x). Additionally, the code was updated to reflect the API change from pymunk.constraint.DampedSpring to pymunk.DampedSpring. The goal is to restore the functionality of the arm simulation within the updated PyMunk environment.
Highlights
- PyMunk Update Compatibility: The code has been modified to work with PyMunk 6.x, addressing coordinate system and API changes.
- Gravity and Orientation Adjustment: The gravity direction and arm orientation have been adjusted to align with the updated PyMunk coordinate system.
- PID Control Tuning: PID control parameters in the gym example have been tuned to accommodate the coordinate system changes.
- API Update: Updated the code to use
pymunk.DampedSpringinstead ofpymunk.constraint.DampedSpringdue to API changes in PyMunk.
Changelog
Click here to see the changelog
- examples/README.md
- Added a section explaining the PyMunk update and the changes made to accommodate it.
- Added notes on API changes and adjustments to gravity and arm orientation.
- examples/envs/pymunk_arm.py
- Changed gravity direction from (0.0, -980.0) to (0.0, 980.0) on line 44.
- Adjusted upper arm angle from np.deg2rad(-45) to np.deg2rad(45) on line 65.
- Flipped lower arm starting angle by negating the value on line 75.
- Adjusted hand y-coordinate to match the new orientation on line 92.
- Updated
pymunk.constraint.DampedSpringtopymunk.DampedSpringon lines 107 and 119. - Flipped elbow stop point to match new orientation on line 134.
- Added a comment about using a moderate gain for balanced muscle power on line 167.
- Modified the debug flag check to use
!=instead ofis noton line 183.
- examples/pymunk-gym-example.py
- Changed hand target y-coordinate to 200 and target delta to -10 on line 29.
- Inverted error calculation for the proportional component of the PID controller on line 49.
- Adjusted signs for the integral and differential components of the PID controller on lines 53 and 56.
- Added print statement for
prev_yto display the excitation required on line 69.
- examples/requirements.txt
- Added requirements file to specify versions of dependencies
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Trivia time!
What is the purpose of the 'sensor' attribute in a PyMunk shape?
Click here for the answer
Setting the 'sensor' attribute to True on a shape in PyMunk disables collisions for that shape. Sensor shapes can be useful for triggering events or gathering information without affecting the physics simulation.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request addresses issues caused by a PyMunk update, specifically coordinate changes that broke the gym example. The changes include adjusting gravity, angles, and hand positions to align with the new coordinate system. Additionally, the code updates the API call for DampedSpring and adjusts PID control parameters to accommodate the flipped coordinate system. The addition of a requirements.txt file is also a good practice for managing dependencies.
Summary of Findings
- Coordinate System Adjustments: The code correctly adjusts gravity, angles, and hand positions to align with the updated PyMunk coordinate system. The comments explaining these adjustments are helpful for understanding the changes.
- API Update: The code updates the API call for
DampedSpringfrompymunk.constraint.DampedSpringtopymunk.DampedSpring, ensuring compatibility with the newer PyMunk version. - PID Control Adjustments: The PID control parameters are adjusted to accommodate the flipped coordinate system, ensuring the arm tracks the target smoothly. The comments explaining the inverted error calculation and velocity calculation are valuable.
Assessment
The pull request effectively addresses the issues caused by the PyMunk update. The code changes are well-commented and the adjustments made to gravity, angles, hand positions, API calls, and PID control parameters seem appropriate. The addition of the requirements.txt file is a good practice. Overall, the pull request is in good shape to be merged, but please have others review and approve this code before merging.
| @@ -54,15 +54,15 @@ def _add_arm(self): | |||
| "brach_stiffness": 450, | |||
| "brach_damping": 200, | |||
| "tricep_rest_length": 30, | |||
| "tricep_stiffness": 50, | |||
| "tricep_stiffness": 200, | |||
There was a problem hiding this comment.
Increasing the tricep stiffness seems like a reasonable adjustment given the coordinate changes. Is there a particular reason why the value was increased to 200?
A pymunk change completely borked how coordinates work and broke the gym example. Here we try to fix it.