Skip to content

Fix handling of std::isnan for IntelLLVM (icpx) with optimisations - #402

Open
wdeconinck wants to merge 2 commits into
developfrom
fix/intel-isnan
Open

wdeconinck wants to merge 2 commits into
developfrom
fix/intel-isnan

Conversation

@wdeconinck

@wdeconinck wdeconinck commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Description

IntelLLVM (icpx) with optimisations automatically assumes std::isnan(value) == false
There are a number of files in eckit that check for std::isnan and unit tests which can fail.

This PR adds compilation flags for IntelLLVM:

  • -fno-finite-math-only: Standard override safe for 2021-2022 and 2025+
  • -fhonor-nans: Plugs the optimization bypass gap specific to 2023.1 - 2024.2

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

🌦️ >> Documentation << 🌦️
https://sites.ecmwf.int/docs/dev-section/eckit/pull-requests/PR-402

@pmaciel pmaciel left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @wdeconinck I think this is great and a good safety net. It's tricky to test as things like std::isnan get optimised out and some logic can be rendered useless, so I wanted tests, in https://github.com/ecmwf/eckit/tree/fix/intel-isnan-tests. Have a look and port and/or cherry-pick?

I'm happily assuming we don't want to support the intel "classic" compilers (19?) (as opposed to the llvm newer ones)

@wdeconinck

Copy link
Copy Markdown
Member Author

@pmaciel I cherry-picked your extra test changes.
Without the compile flags I get for a Release build with icpx-2023.2 the following failures, which disappear with this PR. Notably some further tests were already failing too it seems.

The following tests FAILED:
	141 - eckit_test_parser_yaml (Failed)
	157 - eckit_test_types_doublecompare (Failed)
	158 - eckit_test_types_floatcompare (Failed)
	163 - eckit_test_types_fraction (Failed)
	226 - eckit_test_geo_projection (Failed)
	233 - eckit_test_geo_projection_space_view (Failed)
141: Test "test_eckit_yaml_20" failed: Condition failed: isinf((double)v["infinity"]) @  (eckit/tests/parser/test_yaml.cc:242 test_235)
157: Test "test_comparisons_involving_infinity" failed: Condition failed: !is_equal(dInf, dMax) @  (eckit/tests/types/test_doublecompare.cc:186 test_180)
157: Test "test_comparisons_involving_nan" failed: Condition failed: !is_equal(qNaN, qNaN) @  (eckit/tests/types/test_doublecompare.cc:213 test_196)
157: Test "test_comparisons_involving_runtime_nan" failed: Condition failed: !is_equal(nan, nan) @  (eckit/tests/types/test_doublecompare.cc:257 test_250)
158: Test "test_comparisons_involving_infinity" failed: Condition failed: !is_equal(dInf, dMax) @  (eckit/tests/types/test_floatcompare.cc:186 test_180)
158: Test "test_comparisons_involving_nan" failed: Condition failed: !is_equal(qNaN, qNaN) @  (eckit/tests/types/test_floatcompare.cc:213 test_196)
158: Test "test_comparisons_involving_runtime_nan" failed: Condition failed: !is_equal(nan, nan) @  (eckit/tests/types/test_floatcompare.cc:257 test_250)
163: Test "Fraction from NaN" failed: Expected exception (AssertionFailed)not thrown in: Fraction{nan} @  (eckit/tests/types/test_fraction.cc:305 test_300)
226: Test "projection: coordinates (section: failure is NaN)" failed: Condition failed: all_nan(fwd(failing, v{-10., 20.})) @  (eckit/tests/geo/projection.cc:422 test_383)
226: Test "projection: vectors (section: failure is NaN, for the failing points only)" failed: Condition failed: std::isnan(xy[0][1]) && std::isnan(xy[1][1]) @  (eckit/tests/geo/projection.cc:473 test_427)
233: Test "space_view: fwd, inv" failed: Condition failed: std::isnan(std::get<PointXY>(sv->fwd(PointLonLat{lon_0 + 180., 0.})).X()) @  (eckit/tests/geo/projection_space_view.cc:69 test_57)

@wdeconinck wdeconinck added the downstream-ci-not-needed Deliberately skip the downstream fan-out label Oct 29, 2026
@wdeconinck

Copy link
Copy Markdown
Member Author

I'm happily assuming we don't want to support the intel "classic" compilers (19?) (as opposed to the llvm newer ones)

We do want to test intel "classic" (ICC,ICPC,IFORT), at least version 2023.2 as well, as that is used in operations still.
That is the very last release I think of the "classic" compilers. Not sure it's worthwhile to support older versions now, unless requested by external users.

The new LLVM-based ones (ICX,ICPX,IFX) is what we will also need to support going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

downstream-ci-not-needed Deliberately skip the downstream fan-out

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants