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

KineticsAddSpecies.add_species_sequential test fails due to strict floating-point value comparison? #433

Closed
boegel opened this issue Feb 15, 2017 · 9 comments

Comments

@boegel
Copy link

boegel commented Feb 15, 2017

Cantera version

2.3.0

Operating System

CentOS 7.3

Python/MATLAB version

  • Python 2.7.12
  • no MATLAB

Expected Behavior

test passes

Actual Behavior

test fails:

[----------] 2 tests from KineticsAddSpecies
[ RUN      ] KineticsAddSpecies.add_species_sequential
test/kinetics/kineticsFromScratch.cpp:432: Failure
      Expected: k_ref[i]
      Which is: 217.9903887967192
To be equal to: k[i]
      Which is: 217.99038879671934
i = 2; N = 4
test/kinetics/kineticsFromScratch.cpp:451: Failure
      Expected: w_ref[iref]
      Which is: 217.9903887967192
To be equal to: w[i]
      Which is: 217.99038879671934
sp = H2O2; N = 4
test/kinetics/kineticsFromScratch.cpp:432: Failure
      Expected: k_ref[i]
      Which is: 219.54703062549422
To be equal to: k[i]
      Which is: 219.54703062549436
i = 2; N = 4
test/kinetics/kineticsFromScratch.cpp:451: Failure
      Expected: w_ref[iref]
      Which is: 219.54703062549422
To be equal to: w[i]
      Which is: 219.54703062549436
sp = H2O2; N = 4
[  FAILED  ] KineticsAddSpecies.add_species_sequential (189 ms)
[ RUN      ] KineticsAddSpecies.add_species_err_first
[       OK ] KineticsAddSpecies.add_species_err_first (2 ms)
[----------] 2 tests from KineticsAddSpecies (191 ms total)

The test seems to fail because the received values are slightly off, but since they are very close I wonder whether this is a problem caused by direct comparison of floating-point values rather than anything else.

Steps to reproduce

  1. build Cantera 2.3.0 from source using scons install env_vars=all CC="icc" CXX="icpc" blas_lapack_libs=mkl_rt blas_lapack_dir=$BLAS_LAPACK_LIB_DIR sundials_include=$EBROOTSUNDIALS/include sundials_libdir=$EBROOTSUNDIALS/lib prefix=$PREFIX
  2. try to run tests with scons test env_vars=all CC="icc" CXX="icpc" blas_lapack_libs=mkl_rt blas_lapack_dir=$BLAS_LAPACK_LIB_DIR sundials_include=$EBROOTSUNDIALS/include sundials_libdir=$EBROOTSUNDIALS/lib
@bryanwweber
Copy link
Member

bryanwweber commented Feb 15, 2017

As it turns out, this was reported last year as #367. Can you try the solutions mentioned there, and report back with the information @speth requested? Thanks!

@boegel
Copy link
Author

boegel commented Feb 15, 2017

@bryanwweber I don't see any solutions being mentioned there? The changed tests that should provide more information are included in Cantera v2.3.0, are they not?

#367 is about building/testing Cantera on POWER, while in my case it's about regular 64-bit x86.

I am using the Intel compilers to compile Cantera, which may explain why this hasn't popped up before.

This issue occurring at two very different platforms seems to confirm that the strict comparison of the floating-point values is the problem here?

@skyreflectedinmirrors
Copy link
Contributor

skyreflectedinmirrors commented Feb 15, 2017

@boegel I believe that this may be related to an issue I had with the intel compilers (not the system itself)

https://groups.google.com/forum/#!topicsearchin/cantera-users/authorname$3A%22nick$20curtis%22/cantera-users/A7UWb9pZe3A

What version of ICC, etc. are you using?

@skyreflectedinmirrors
Copy link
Contributor

Although you are correct that the discrepancies you see are of much smaller magnitude, suggesting that perhaps it's an issue due to floating point comparison.

Maybe try supplying the -fp-model precise to the intel compilers?

@bryanwweber
Copy link
Member

@boegel Thanks, and sorry I didn't read this issue closely enough. In addition to what @arghdos suggested, do you get the same errors if you use GCC? One question that might be useful from the other issue is

Has the test suite been running successfully previously on this platform? If so, can you identify the commit where the test started failing?

Thanks!

@speth
Copy link
Member

speth commented Feb 15, 2017

Obviously this test would succeed if we allowed for inexact floating point comparison. As I mentioned in the other issue, the problem is that this is a case where I think that the two values being compared should in fact be identical, since they should be computed by running through exactly the same functions, The usual reasons for variations in floating point results like how operations are grouped or when/if intermediate variables are truncated should not apply.

I'll second the question about exactly which version of the Intel compiler is being used. Cantera is regularly tested with ICPC 14.0.1 (2013), so it's not a general issue with the Intel compiler. I would also be interested to know whether or there's an issue with using GCC on the same system, to see if the problem is related to any of the system libraries.

@boegel
Copy link
Author

boegel commented Feb 16, 2017

The Intel compilers being used are:

Intel(R) C Intel(R) 64 Compiler for applications running on Intel(R) 64, Version 16.0.3.210 Build 20160415

This is the first time I build Cantera 2.3.0, so no success with earlier builds with this version.
I did install Cantera 2.2.1 without problems however, with the same version of the Intel compilers, and that build still succeeds now.

I am already using -fp-model precise, i.e. both $CFLAGS and $CXXFLAGS are defined as -O2 -xHost -fp-speculation=strict -fp-model strict in the build environment. Is this sufficient when building with scons install env_vars=all, or do they need to be passed down explicitly somehow?

I'll try also building Cantera 2.3.0 with GCC v5.4.0; I'll get back to you on that.

We try to take control over all dependencies (e.g. Boost, SUNDIALS, Eigen, ...) via EasyBuild (http://hpcugent.github.io/easybuild/), which also get built with the same compilers & libraries as Cantera is.

speth added a commit to speth/cantera that referenced this issue Feb 22, 2017
Exact floating point equality can be assured only in the case where the species
are added in the same order, since this affects summations involved in
calculating the mixture molecular weight. This resulted in test failures with
certain versions of the Intel compiler.

Resolves Cantera#433.
@speth
Copy link
Member

speth commented Feb 22, 2017

I believe the issue with this test should be resolved by this commit: 11a0727, if you would like to confirm.

SCons does not use environment variables such as CFLAGS and CXXFLAGS, since that is just a convention used by make. Instead, you should set the values of the options cc_flags, cxx_flags and company.

@boegel
Copy link
Author

boegel commented Feb 22, 2017

@speth The test is indeed fixed with 11a0727, so I'm closing this, thanks!

@boegel boegel closed this as completed Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants