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

HDF5 FPE fixes. #408

Merged

Conversation

rhoneyager-tomorrow
Copy link

@rhoneyager-tomorrow rhoneyager-tomorrow commented Feb 27, 2024

Description

This fixes a bunch of HDF5 floating point exception issues. This patch is backported from HDF5's develop branch. See code notes, and see HDFGroup/hdf5#3837 / spack#42880.

@climbfuji climbfuji self-assigned this Feb 27, 2024
@climbfuji climbfuji added the INFRA JEDI Infrastructure label Feb 27, 2024
@climbfuji
Copy link
Collaborator

I am building spack-stack for JCSDA/spack-stack#1014, which includes this PR in the submodule ptr update. Then I'll run the jedi-bundle ctests to confirm that it solves the FPE issues we've been seeing.

Thanks for the PR!

Copy link
Collaborator

@srherbener srherbener left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! I think the approach the HDF developers took (with the fenv utilities) covers all the platforms we are using except for the Intel based macs. The Intel based mac is actually partly covered by fenv. For the Intel based macs, the fenv utilities cover the "regular" arithmetic pipelines, but not the SSE arithmetic pipelines. The SSE is handled by special macros in xmmintrin.h. Here's an example from oops:

https://github.com/JCSDA/oops/blob/2012398058043bda46db2b9d80a909810df04cea/src/oops/util/signal_trap.cc#L116-147

At this point the Intel based macs are disappearing, and we will eventually stop supporting that platform. Nearly everyone has an M1 or M2 based mac, so I'm not sure it's worth trying to get this into the HDF code.

Just wanted to point this out.

@climbfuji
Copy link
Collaborator

Thanks for this PR! I think the approach the HDF developers took (with the fenv utilities) covers all the platforms we are using except for the Intel based macs. The Intel based mac is actually partly covered by fenv. For the Intel based macs, the fenv utilities cover the "regular" arithmetic pipelines, but not the SSE arithmetic pipelines. The SSE is handled by special macros in xmmintrin.h. Here's an example from oops:

https://github.com/JCSDA/oops/blob/2012398058043bda46db2b9d80a909810df04cea/src/oops/util/signal_trap.cc#L116-147

At this point the Intel based macs are disappearing, and we will eventually stop supporting that platform. Nearly everyone has an M1 or M2 based mac, so I'm not sure it's worth trying to get this into the HDF code.

Just wanted to point this out.

Thanks for the comment. Let's get this merged and deal with the Intel-based macs on our end if we have to.

@climbfuji climbfuji merged commit 8e40b08 into JCSDA:jcsda_emc_spack_stack Feb 28, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INFRA JEDI Infrastructure
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

4 participants