mutationpp / Mutationpp

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

Update electron molecular weight. #211

Closed mgoodson-cvd closed 1 year ago

mgoodson-cvd commented 2 years ago

Closes #210. Updates the electron molecular weight to be more precise. Uses the value given by the NASA-9 thermodynamics database. Also updates the unit tests with the same value.

Fixes a minor bug with an undefined return type in XMLite.

Marking as draft because the comparisonTest unit test is still failing due to the use of old data. The comparison data will either need to be regenerated or the tolerance increased.

jbscoggi commented 2 years ago

Hey @mgoodson-cvd, It seems like some regression tests are failing, likely due to the change in the the electron molecular weight. We just need to update the regression tests to use the new molecular weight value. I'll come back to you with the procedure for doing this as soon as I remember how!

jbscoggi commented 2 years ago

@capriatim, I think you fixed the electron molecular weight in the past for US3D, can you check if this change is consistent with yours?

mgoodson-cvd commented 2 years ago

@jbscoggi I went digging and found @capriatim's commit here. Our updated values for the electron molecular weight actually do match. @capriatim also updated the molecular weight of carbon; I am happy to tack that on to this PR, or we can leave it for a separate issue.

mgoodson-cvd commented 1 year ago

@jbscoggi Did you ever find your tool to generate the comparison unit test data? If it's long gone I can try to reverse-engineer it.

mgoodson-cvd commented 1 year ago

@jbscoggi I found the update_comparison code and figured out how to use it to update the comparison data for the unit tests. I've added those and force pushed.

I also went ahead and updated the version number because this will cause users' answers to change.

mgoodson-cvd commented 1 year ago

Also, the unit test failures don't look to be my fault. Because you're using ubuntu-latest for the CI/CD, clang++ has updated recently and variable-length arrays (which you use in M++) now cause build errors.

mgoodson-cvd commented 1 year ago

Correction: the compiler error due to variable length arrays is coming from Catch, not Mutation++. I've replicated the ubuntu-latest build environment with clang 14, working off Mutation++ master branch. I'll see if simply updating Catch will solve this.

codecov[bot] commented 1 year ago

Codecov Report

Merging #211 (32c8c12) into master (4a3ee5c) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #211   +/-   ##
=======================================
  Coverage   71.48%   71.48%           
=======================================
  Files         135      135           
  Lines        9114     9114           
=======================================
  Hits         6515     6515           
  Misses       2599     2599           
mgoodson-cvd commented 1 year ago

Closes #229. The ubuntu-latest builds are all successful after updating catch2.

Closes #230. The python 3.11 build is successful after updating pybind11.

Looks like your runners aren't picking up the ubuntu-18.04 builds. I would consider dropping those and just using ubuntu-latest, at which point this PR will be good to go.