Skip to content

Fixed HOverRT in python wrapper#265

Closed
Fratorhe wants to merge 2 commits into
mutationpp:masterfrom
Fratorhe:fix_enthalpy_python
Closed

Fixed HOverRT in python wrapper#265
Fratorhe wants to merge 2 commits into
mutationpp:masterfrom
Fratorhe:fix_enthalpy_python

Conversation

@Fratorhe
Copy link
Copy Markdown
Contributor

@Fratorhe Fratorhe commented May 14, 2024

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

…le of additional functions with their tests for "existance".
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

❗ 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
@@           Coverage Diff           @@
##           master     #265   +/-   ##
=======================================
  Coverage   71.65%   71.65%           
=======================================
  Files         135      135           
  Lines        9089     9089           
=======================================
  Hits         6513     6513           
  Misses       2576     2576           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mgoodson-cvd
Copy link
Copy Markdown
Collaborator

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
Copy link
Copy Markdown
Contributor Author

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
Copy link
Copy Markdown
Collaborator

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

@BBArrosDias BBArrosDias mentioned this pull request Jul 6, 2024
@rdbisme rdbisme closed this in #269 Jul 6, 2024
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

Successfully merging this pull request may close these issues.

3 participants