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

CI: Update macOS Runner (Latest) #54

Merged
merged 12 commits into from
Aug 30, 2024
Merged

CI: Update macOS Runner (Latest) #54

merged 12 commits into from
Aug 30, 2024

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Aug 26, 2024

The old macos-11 image is now removed. Update to running release macos-latest.

  • picsar_tables test: some numerical values vary on near-machine precision but compare without an epsilon

@ax3l ax3l requested a review from lucafedeli88 August 26, 2024 16:04
@ax3l ax3l added the enhancement New feature or request label Aug 26, 2024
@ax3l ax3l force-pushed the ci-macos-latest branch 2 times, most recently from f3aa671 to 1528ec2 Compare August 26, 2024 16:14
The old `macos-11` image is now removed. Update to
running release `macos-latest`.
@ax3l ax3l force-pushed the ci-macos-latest branch from 1528ec2 to 9195224 Compare August 26, 2024 16:48
@ax3l
Copy link
Member Author

ax3l commented Aug 26, 2024

@lucafedeli88 the table test seems to need a tolerance review on the last significant digits. Can you help with this? 🙏

Copy link
Member

@lucafedeli88 lucafedeli88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ax3l ,
thanks for this PR!
Precision for the failing test (test_picsar_tables.cpp) is set by these two variables

//Tolerance for double precision calculations
const double double_tolerance = 1.0e-12;

//Tolerance for single precision calculations
const float float_tolerance = 1.0e-5;

We may try to increase the tolerance by a small factor (e.g., 2 or 5) and see if the tests pass.
I can do that if you want!
Besides of that, the PR looks good to me: as soon as the tests pass we can merge it.

@ax3l
Copy link
Member Author

ax3l commented Aug 29, 2024

Hi @lucafedeli88,

We may try to increase the tolerance by a small factor (e.g., 2 or 5) and see if the tests pass.
I can do that if you want!

Thanks a lot - I pushed a few updates, but happy if you can take it over for the tolerances :)

@ax3l
Copy link
Member Author

ax3l commented Aug 29, 2024

Ah, the tolerances are not used in the tests that actually fail:

  • BOOST_CHECK_EQUAL for the linear_function

@ax3l ax3l requested a review from lucafedeli88 August 29, 2024 17:05
Copy link
Member

@lucafedeli88 lucafedeli88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry! I didn't know that the failing test actually used BOOST_CHECK_EQUAL.
I think that your change makes sense here.
Thanks for this PR!

@lucafedeli88 lucafedeli88 merged commit 6b46dbe into development Aug 30, 2024
8 checks passed
@ax3l ax3l deleted the ci-macos-latest branch September 24, 2024 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants