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

fix: Issue 534 transformation tests failing on some platforms #559

Merged

Conversation

boxanm
Copy link
Collaborator

@boxanm boxanm commented Jan 23, 2024

Description

Summary:

The 3D Transformation utests were failing on Ubuntu Bionic with Eigen 3.3.4, in contrast to all other platforms tested with Eigen 3.4.
I added a non-zero epsilon precision value to Eigen's isApprox call for Rotation and Rotation+Scaling tests.
My guess is the two versions of Eigen treat zero epsilon precision values differently; hence the tests weren't failing with 3.4.0.
Fixes #534

Changes and type of changes (quick overview):

  • Added a non-zero epsilon precision value to Eigen's isApprox in failing Transformation utest.

Checklist:

Code related

  • I have made corresponding changes to the documentation
    (i.e.: function, class, script header, README.md)
  • I have commented hard-to-understand code
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally with my changes
    (Check README.md #Contributing
    for local testing procedure using libpointmatcher-build-system)

PR creation related

  • My pull request base ref branch is set to the develop branch
    (the build-system won't be triggered otherwise)
  • My pull request branch is up-to-date with the develop branch
    (the build-system will reject it otherwise)

PR description related

  • I have included a quick summary of the changes
  • I have indicated the related issue's id with # <issue-id> if changes are of type fix
  • I have included a high-level list of changes and their corresponding type
    • Types: feat fix docs style refactor perf test build ci chore revert
    • Breaking changes: <type>!
    • Reference:
      see commit_msg_reference.md
      in the repository root for details

Note for repository admins

Release PR related

  • Only repository admins have the privilege to push/merge on the default branch (ie: master)
    and the release branch.
  • Keep PR in draft mode until all the release reviewers are ready to push the release.
  • Once a PR from release -> master branch is created (not in draft mode),
    • it triggers the build-system test
    • (in-progress) and it triggers the semantic release automation

Copy link

sonarcloud bot commented Jan 29, 2024

Quality Gate Failed Quality Gate failed

Failed conditions

20.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@boxanm
Copy link
Collaborator Author

boxanm commented Jan 31, 2024

The arm64 build randomly falls on the build farm. Since this issue wasn't encountered locally on an arm machine, we assume it was due to the CPU architecture virtualization.
I'm therefore bypassing the branch protections.

@boxanm boxanm merged commit 77bd7ab into develop Jan 31, 2024
4 of 6 checks passed
@boxanm boxanm deleted the issue_534-transformation-tests-failing-on-some-platforms branch January 31, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants