-
Notifications
You must be signed in to change notification settings - Fork 111
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
Update FixedJoints: restoring spring-damped-torques, initial rotation offset #135
Update FixedJoints: restoring spring-damped-torques, initial rotation offset #135
Conversation
…otation errors using rotational spring-damper systems
@xzhan139 What is the reason that the damping force for the relative_velocity = (
rod_two.velocity_collection[..., index_two]
- rod_one.velocity_collection[..., index_one]
)
normal_relative_velocity = (
np.dot(relative_velocity, normalized_end_distance_vector)
* normalized_end_distance_vector
)
damping_force = -self.nu * normal_relative_velocity When adding damping to the torque compensation for the |
Codecov Report
@@ Coverage Diff @@
## update-0.3.0 #135 +/- ##
================================================
- Coverage 87.30% 87.24% -0.07%
================================================
Files 44 44
Lines 2695 2705 +10
Branches 360 360
================================================
+ Hits 2353 2360 +7
- Misses 324 326 +2
- Partials 18 19 +1
Continue to review full report at Codecov.
|
@mstoelzle after discussing your question with @xzhan139, the conclusion is that there is no need to project it onto the normal vector. You can use directly the relative velocity field. In reality, contact damping is optional, since the internal dissipation of the rod helps in stabilizing the simulations.
After you have done all necessary changes to the PR, please let us know or remove the WIP keyword from the PR name to open it for review. |
- remove static rotation from test - Improve checking of nut damping - Fix sign error for computing restoring rotational damping torque
@bhosale2 This PR is now ready for review |
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.
I only have minor comments. I think the implementation is not wrong but not so clear. @bhosale2 Please take a look.
I would suggest avoiding any branching, and make FixedJoint
with rotational offset as a default behavior. @armantekinalp
If we need branching (static_rotation
), we should have separate test for each cases.
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.
good job overall @mstoelzle, some critical comments to be addressed.
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.
Good job @mstoelzle minor changes.
Co-authored-by: Arman Tekinalp <[email protected]>
Co-authored-by: Arman Tekinalp <[email protected]>
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.
LGTM. @skim0119 and @armantekinalp will follow up.
@mstoelzle some CI tests are failing can you merge changes from |
Thanks. I will do that. There is also still some test issues I want to have a look at. |
…com/tud-cor-sr/PyElastica into bugfix/fixed-joint-torsional-torque
…d-joint-torsional-torque
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.
It is probably the last set of changes. After you fix them, I'll run the testings and JointCases
code, and approve.
…o use `rest_rotation_matrix`
Co-authored-by: Seung Hyun Kim <[email protected]>
Co-authored-by: Arman Tekinalp <[email protected]>
…com/tud-cor-sr/PyElastica into bugfix/fixed-joint-torsional-torque
Thanks everybody for your feedback. I have addressed all the requests for changes (in case I didn't miss anything) and this PR should be ready for merging now. |
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.
LGTM, nice work @mstoelzle
Fixes #131 for the FixedJoint class:
kt
scales the rotational frame deviation between systems one and twonut
scales the difference between angular velocities of system one and twouse_static_rotation==False
, the static rotation is set to an identity matrix and restoring torques are applied to fully align the local frames of both systems at the join.examples/JointCases/fixed_joint_torsion.py
illustrates how this implementation generates torsional / twisting torques to prevent torsional deviationsThe attached plots demonstrates the results of the
examples/JointCases/fixed_joint_torsion.py
example: