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

Fixed HOverRT in python wrapper #265

Closed Fratorhe closed 1 week ago

Fratorhe commented 2 months ago

Hi,

I realized of an error in the python wrapper where the function speciesHOverRT was actually calling the M++ method speciesCpOverR instead. I added that function with at the reference temperature and the overloaded with H(T). I fixed this and added a test for it. I also added a couple of convenience functions (convert_xe_to_ye and convert_ye_to_xe) that were not in the wrapper yet. The tests for these two functions are only for "existence" verification with dummy inputs.

Best, Fran

codecov[bot] commented 2 months ago

Codecov Report

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

Project coverage is 71.65%. Comparing base (aa01785) to head (e8ddb28).

:exclamation: Current head e8ddb28 differs from pull request most recent head 22d69cb. Consider uploading reports for the commit 22d69cb to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #265 +/- ## ======================================= 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 2 months ago

Looking at why this failed on a piece of code you didn't touch, and only for Mac build. The CI/CD for GitHub recently updated MacOS from 12.7.3 to 14.4.1. This also updated AppleClang from 14.0 to 15.0. Without seeing the actual failure log, I would guess this OS/Compiler change shifts the numerics just enough to trigger failure. Presumably master branch would also fail if we re-ran the testing now.

If I could see the test log for the Mac build (or if I had a Mac), I could see which check needs a little more wiggle room; but unfortunately I can't.

Fratorhe commented 2 months ago

Yes, indeed, I also found it weird that it failed for MacOS. I am on Linux and I do not have access to any MacOS machine...

BBArrosDias commented 3 weeks ago

I am adding some other functions to the python wrapper and I will include these.