scpeters commented 6 months ago

🦟 Bug fix

Fixes https://github.com/gazebosim/gz-physics/issues/612

Summary

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

This reverts commit d616379c6875f04527b1761ecdd7633155e17138.

The ode bottle should be fixed by https://github.com/Homebrew/homebrew-core/pull/167721

Checklist

[X] 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)
[X] codecheck passed (See contributing)
[X] 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.

scpeters commented 6 months ago

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

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

:exclamation: 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

```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

scpeters commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

I am afraid I indirectly caused this. See https://bitbucket.org/odedevs/ode/issues/87/box-cylinder-ccd-check-disabled-by-default and https://github.com/conda-forge/dartsim-feedstock/issues/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 commented 6 months ago

fyi @jslee02

scpeters commented 6 months ago

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

https://build.osrfoundation.org/view/gz-harmonic/job/gz_physics-ci-gz-physics7-homebrew-amd64/34/testReport/junit/(root)/JointFeaturesDetachTest_0/JointDetach/

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 commented 5 months ago

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

https://build.osrfoundation.org/view/gz-harmonic/job/gz_physics-ci-gz-physics7-homebrew-amd64/34/testReport/junit/(root)/JointFeaturesDetachTest_0/JointDetach/

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 https://github.com/gazebosim/gz-physics/pull/621

gazebosim / gz-physics

Revert workaround for homebrew ODE issue #615

🦟 Bug fix

Summary

Checklist

Codecov Report