search

mutationpp / Mutationpp

The MUlticomponent Thermodynamic And Transport library for IONized gases in C++
GNU Lesser General Public License v3.0
101 stars 58 forks source link

Revert "Closes #258. Remove `use_transport` flag" #267

Open BBArrosDias opened 3 weeks ago

BBArrosDias commented 3 weeks ago

Reverts mutationpp/Mutationpp#259

The Build / build_mac tests have failed.

@mgoodson-cvd do you have a Mac?

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.65%. Comparing base (bc221f4) to head (3bb3a5f). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #267 +/- ## ======================================= Coverage 71.65% 71.65% ======================================= Files 135 135 Lines 9089 9089 ======================================= Hits 6513 6513 Misses 2576 2576 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mgoodson-cvd commented 3 weeks ago

No, but this same issue also affects #265; see this comment. My guess is that it is this line; it probably needs a tolerance on this check. I actually had to do this myself when compiling M++ with old Intel 2018 compilers. I changed it to:

CHECK(model.relaxationTime(mix) == Approx(tau_mw + tau_park).epsilon(tol));

with const double tol = 100*std::numeric_limits<double>::epsilon();. (Maybe could even be constexpr? Didn't try).

Again, I don't have a Mac, so I can't test my hypothesis. I would suggest making this change and re-running pipeline before reverting the merge. There is nothing wrong with the merge itself, as it doesn't affect the M-W code at all. I can make a PR with this change if you'd like, that will trigger a pipeline, and if it passes, then we're good.