Cantera / cantera

Chemical kinetics, thermodynamics, and transport tool suite
https://cantera.org
Other
582 stars 342 forks source link

Fix problems in ThermoPhase and Kinetics classes for the experimental Matlab toolbox #1586

Closed ssun30 closed 8 months ago

ssun30 commented 10 months ago

Changes proposed in this pull request

If applicable, fill in the issue number this pull request is fixing

Partially address Cantera/enhancements#177

If applicable, provide an example illustrating new features this pull request is introducing

PR #1496 introduced a test suite to the experimental Matlab interfaces. Currently, 12 out of 26 tests implemented in the test suite for the experimental Matlab interface are marked as filtered since they could not pass. This PR will address these problems so they will pass. Some changes to the Clib may be required as some of these problems are not related to the Matlab side of the interface.

Checklist

codecov[bot] commented 10 months ago

Codecov Report

Merging #1586 (e213360) into main (de77fa3) will decrease coverage by 0.02%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1586      +/-   ##
==========================================
- Coverage   72.72%   72.70%   -0.02%     
==========================================
  Files         370      370              
  Lines       56286    56298      +12     
  Branches    20369    20375       +6     
==========================================
  Hits        40932    40932              
- Misses      12357    12369      +12     
  Partials     2997     2997              
Files Coverage Δ
src/clib/ct.cpp 18.87% <0.00%> (-0.23%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

ssun30 commented 9 months ago

Sorry I've been busy with class and proposal defense in the past few weeks. I just got some time to go over the PR again. I mentioned to Ray earlier that some methods could not return correct error messages. It turns out these are methods that return certain indices such as thermo_speciesIndex and thermo_elementIndex.

They do not throw exceptions when the returned values are out of bounds (npos). Instead, there are dedicated methods to check whether these indices are out of bounds, for example Phase::checkSpeciesIndex. From my understanding, it was done this way so that three-body reactions (with undefined species 'M') do not cause errors when they are imported. Therefore I've moved the error handling into the Clib layer.

The other remaining issue with the ThermoPhase class in the Matlab interface is that multi-property setters (such as ThermoPhase.TDY may set incorrect values. I couldn't pinpoint the cause just yet and will test it on Thursday.