libantioch / antioch

C++ Chemical Kinetics, Thermodynaimics, and Transport Library
https://libantioch.github.io/
Other
22 stars 17 forks source link

NASAThermo Vectorized Evaluations Fix, More Unit Testing #201

Closed pbauman closed 8 years ago

pbauman commented 8 years ago

Builds on #200.

Went down this rabbit hole when removing CEAThermodynamics (that's the next PR after this one). The NASAThermoMixture/Evaluators and, therefore, the underlying curve fits, were not exercised in unit tests using vectorized types. This PR makes several fixes to allow that and adds unit testing (in the CppUnit testing framework) for valarray, Eigen, and VexCL types for the NASAThermo. I'll note in the testing, I made sure that the temperatures were such that two different intervals will be need. I'm pretty sure there was a bug in the old CEAThermodynamics implementation for vectorized types that we didn't see because we were always in the same temperature interval in the test.

All tests pass for me with Eigen 3.2.8 and VexCL 1.3.3 using GCC 4.9.3. GCC 5.3 has failures in a few of the Eigen tests (nans).

pbauman commented 8 years ago

So, this is in the raw log for the Clang build (GCC passed):

The log length has exceeded the limit of 4 MB (this usually means that the test suite is raising the same exception over and over).
The job has been terminated

It looks like there are so many 'binder2nd' is deprecated [-Wdeprecated-declarations] warnings coming out of Eigen, when compiled with Clang, that it's overflowing the log file...

pbauman commented 8 years ago

Apple's Clang on my laptop is actually generating errors:

In file included from ../../../pbauman/test/hercourtessen_rate_vec_unit.C:51:
../../../pbauman/src/kinetics/include/antioch/hercourtessen_rate.h:109:39: error: no matching function for call to 'ant_pow'
    ANTIOCH_AUTOFUNC(StateType, _Cf* (ant_pow(T,_eta)))
                                      ^~~~~~~
../../../pbauman/src/utilities/include/antioch/metaprogramming_decl.h:52:47: note: expanded from macro 'ANTIOCH_AUTOFUNC'
#define ANTIOCH_AUTOFUNC(Type, Expr) { return Expr; }
                                              ^
../../../pbauman/src/kinetics/include/antioch/hercourtessen_rate.h:115:39: note: in instantiation of function template specialization 'Antioch::HercourtEssenRate::rate >' requested here
    ANTIOCH_AUTOFUNC(StateType, this->rate(T))
                                      ^
../../../pbauman/src/utilities/include/antioch/metaprogramming_decl.h:52:47: note: expanded from macro 'ANTIOCH_AUTOFUNC'
#define ANTIOCH_AUTOFUNC(Type, Expr) { return Expr; }
                                              ^
../../../pbauman/test/hercourtessen_rate_vec_unit.C:191:10: note: in instantiation of function template specialization 'Antioch::HercourtEssenRate::operator() >' requested here
  rate = hercourtessen_rate(T);
         ^
../../../pbauman/test/hercourtessen_rate_vec_unit.C:222:5: note: in instantiation of function template specialization 'vectester >' requested here
    vectester (std::valarray(2*ANTIOCH_N_TUPLES), "valarray");
    ^
../../../pbauman/src/utilities/include/antioch/cmath_shims.h:115:1: note: candidate template ignored: substitution failure [with T1 = std::__1::valarray, T2 = float]: call to 'pow' is ambiguous
ANTIOCH_BINARY_SHIM(pow)
^                   ~~~
../../../pbauman/src/utilities/include/antioch/cmath_shims.h:67:3: note: expanded from macro 'ANTIOCH_BINARY_SHIM'
  ant_##funcname (const T1& in1, const T2& in2) \
  ^
:200:1: note: expanded from here
ant_pow
^
pbauman commented 8 years ago

OK, rebased. Let's see if this makes Travis happier. Re: Apple Clang failure, I think it might be that version of Clang is too old? I guess we'll see. I don't have any other Clang builds handy at the moment.

pbauman commented 8 years ago

Alright, we're green again! There are a few warnings emitted in the Clang build:

/usr/include/eigen3/Eigen/src/Core/arch/SSE/PacketMath.h:510:3: warning: 
      'register' storage class specifier is deprecated [-Wdeprecated-register]
  register int aux0 = aux[0] < aux[1] ? aux[0] : aux[1];
  ^

I'll leave these on since it's an Eigen warning and it's not overflowing the log file. Hopefully with newer versions of Eigen that will go away.

There's several of the following which I'll fixup in a different PR:

constant_rate_unit.C:39:9: warning: using integer absolute value function 'abs'
      when argument is of floating point type [-Wabsolute-value]
    if( abs( (rate - rate_exact)/rate_exact ) > tol )
        ^
constant_rate_unit.C:83:19: note: in instantiation of function template
      specialization 'check_rate_and_derivative' requested here
    return_flag = check_rate_and_derivative(rate_exact,derive_exact,rate...
                  ^
constant_rate_unit.C:114:17: note: in instantiation of function template
      specialization 'test_values' requested here
  return_flag = test_values(Cf,constant_rate) || return_flag;
                ^
constant_rate_unit.C:130:11: note: in instantiation of function template
      specialization 'tester' requested here
  return (tester() ||
          ^
constant_rate_unit.C:39:9: note: use function 'std::abs' instead
    if( abs( (rate - rate_exact)/rate_exact ) > tol )
        ^~~
        std::abs
pbauman commented 8 years ago

OK, using the raw ints worked, thanks! Good to merge once Travis is happy?

roystgnr commented 8 years ago

Looks great to me, thanks! :+1: