Skip to content
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

Revert workaround for homebrew ODE issue #615

Merged
merged 5 commits into from
Apr 11, 2024

Conversation

scpeters
Copy link
Member

@scpeters scpeters commented Apr 2, 2024

🦟 Bug fix

Fixes #612

Summary

Revert "Disable check in DetachableJointTest, CorrectAttachmentPoints for dartsim plugin on macOS (#613)"

This reverts commit d616379.

The ode bottle should be fixed by Homebrew/homebrew-core#167721

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

… for dartsim plugin on macOS (#613)"

This reverts commit d616379.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
Copy link
Member Author

scpeters commented Apr 2, 2024

ok, this isn't working; I'm looking into it

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.35%. Comparing base (7d30e47) to head (1eca917).
Report is 6 commits behind head on gz-physics7.

❗ Current head 1eca917 differs from pull request most recent head 908cbcc. Consider uploading reports for the commit 908cbcc to get more accurate results

Additional details and impacted files
@@               Coverage Diff               @@
##           gz-physics7     #615      +/-   ##
===============================================
+ Coverage        78.32%   78.35%   +0.02%     
===============================================
  Files              140      140              
  Lines             8069     8079      +10     
===============================================
+ Hits              6320     6330      +10     
  Misses            1749     1749              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@scpeters
Copy link
Member Author

scpeters commented Apr 2, 2024

it has a different test failure than the one reported in #612:

75: [ RUN      ] JointFeaturesDetachTest/0.JointDetach
75: JointFeaturesTest::GetLibToTest() /Users/jenkins/jenkins-agent/workspace/gz_physics-ci-pr_any-homebrew-amd64/build/lib/libgz-physics7-dartsim-plugin.7.1.0.dylib
75: Testing plugin: gz::physics::dartsim::Plugin
75: Warning [Utils.cc:119] [/sdf/world[@name="test_world"]/model[@name="screw_joint_test"]/joint[@name="j0"]/thread_pitch]: SDF Element[thread_pitch] is deprecated
75: [Err] [SDFFeatures.cc:1164] Asked to construct a joint of sdf::JointType [7], but that is not supported yet. Creating a FIXED joint instead
75: [Err] [SDFFeatures.cc:1164] Asked to construct a joint of sdf::JointType [4], but that is not supported yet. Creating a FIXED joint instead
75: [Err] [SDFFeatures.cc:1164] Asked to construct a joint of sdf::JointType [2], but that is not supported yet. Creating a FIXED joint instead
75: /Users/jenkins/jenkins-agent/workspace/gz_physics-ci-pr_any-homebrew-amd64/gz-physics/test/common_test/joint_features.cc:969: Failure
75: The difference between 0.0 and upperLinkLinearVelocity.X() is 0.00035244933516466117, which exceeds 1e-6, where
75: 0.0 evaluates to 0,
75: upperLinkLinearVelocity.X() evaluates to -0.00035244933516466117, and
75: 1e-6 evaluates to 9.9999999999999995e-07.
75: /Users/jenkins/jenkins-agent/workspace/gz_physics-ci-pr_any-homebrew-amd64/gz-physics/test/common_test/joint_features.cc:971: Failure
75: The difference between 0.0 and upperLinkAngularVelocity.Y() is 0.00016783269763408969, which exceeds 1e-6, where
75: 0.0 evaluates to 0,
75: upperLinkAngularVelocity.Y() evaluates to -0.00016783269763408969, and
75: 1e-6 evaluates to 9.9999999999999995e-07.
75: Msg [NameManager::issueNewName] (Skeleton::Joint | double_pendulum_with_base) The name [Joint] is a duplicate, so it has been renamed to [Joint(1)]
75: [  FAILED  ] JointFeaturesDetachTest/0.JointDetach, where TypeParam = JointFeatureDetachList (142 ms)

@iche033
Copy link
Contributor

iche033 commented Apr 2, 2024

There is this note in ODE Changelog:

To obtain the old behavior configure as 
          "--enable-libccd --with-box-cylinder=default".

Not sure if it'll make any difference

@scpeters
Copy link
Member Author

scpeters commented Apr 4, 2024

There is this note in ODE Changelog:

To obtain the old behavior configure as 
          "--enable-libccd --with-box-cylinder=default".

Not sure if it'll make any difference

I want to visualize and compare what's happening with the collisions / contact in this test to see if one configuration option appears more correct than the other

@traversaro
Copy link
Contributor

I am afraid I indirectly caused this. See https://bitbucket.org/odedevs/ode/issues/87/box-cylinder-ccd-check-disabled-by-default and conda-forge/dartsim-feedstock#62 for the context. Basically I had failures in conda-forge as ode build there was failing as CMake was used and the default cmake behaviour was different from the default autotools behavior. When I noticed the problem I reported upstream, that changed it to make autotools default coherent with cmake, that was what was creating dart and gz-physics tests to fail.

@traversaro
Copy link
Contributor

fyi @jslee02

@scpeters
Copy link
Member Author

scpeters commented Apr 4, 2024

the new test failure is also failing on the gz-physics7 branch:

I'll disable the failing expectation in a separate PR to keep things clear, and then I think this will be good to go

@scpeters
Copy link
Member Author

scpeters commented Apr 8, 2024

the new test failure is also failing on the gz-physics7 branch:

I'll disable the failing expectation in a separate PR to keep things clear, and then I think this will be good to go

tracking the new test failure in #620, disabling the failing test expectations for now in #621

@scpeters scpeters merged commit 624dc27 into gz-physics7 Apr 11, 2024
8 checks passed
@scpeters scpeters deleted the scpeters/revert_ode_workaround branch April 11, 2024 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

DetachableJointTest.CorrectAttachmentPoints dartsim test fails on Homebrew CI
3 participants