Pressio / pressio

core C++ library
Other
45 stars 7 forks source link

ci: add c++20 builds #434

Closed mzuzek closed 2 years ago

mzuzek commented 2 years ago

Fixes #429

Status

mzuzek commented 2 years ago

@fnrizzi Do you recall any FP flag(s) in Intel compiler that the test failure can be related to ? We don't control the tolerance on that FP comparison - see https://google.github.io/googletest/reference/assertions.html#floating-point.

fnrizzi commented 2 years ago

this is nice ! thanks so much! This is weird... because the trial_subspace_create_full_from_reduced passes and it is doing exactly the same thing. I think I am going to make a PR where I change EXPECT_DOUBLE_EQ to EXPECT_NEAR .

How about we just use a preproc directive there to guard for intel compiler and use a different check? Something like:

#ifdef PRESSIO_COMPILER_INTEL
  EXPECT_NEAR(a[i], gold[i], 1e-14);
#else 
  EXPECT_DOUBLE_EQ(a[i], gold[i]);
#endif

Note that this PRESSIO_COMPILER_INTEL is defined in the pressio_cmake_config.h but we need to verify that is on for this intel oneapi.

I think we need to leave this pending and address #435 first

Nevremnd , please see this https://github.com/Pressio/pressio/issues/437

mzuzek commented 2 years ago

@fnrizzi

I'm rebasing this PR on #438 which is supposed to fix the failing tests (which provides partial #437 fix replacing EXPECT_DOUBLE_EQ with EXPECT_NEAR where appropriate), so let me change it to draft until #438 is reviewed and merged.

mzuzek commented 2 years ago

Thank you @fnrizzi and @marcinwrobel1986 for your reviews!