Complete Unit Test Suite for the Matlab Toolbox #1665
Conversation
ssun30
force-pushed
the
matlab_patches
branch
from
8a95e99 to
ecc6173
Compare
|
@ssun30 and @rwest ... what are the current plans on this PR and Cantera/enhancements#177? With the old MATLAB toolbox removed, the next Cantera version will only provide for the experimental toolbox. |
Hi Ingmar, finishing the test suite is back on our to-do list and we were aiming to complete it by the end of the month. There is a slight delay so maybe the first week of July we shall have all the unit tests ready. |
ssun30
force-pushed
the
matlab_patches
branch
3 times, most recently
from
8c98850 to
3acff45
Compare
|
After a year of hiatus this PR is finally ready for review. Thanks to support from Mathworks the plan to make the Matlab toolbox official is back on track. |
ssun30
force-pushed
the
matlab_patches
branch
from
69db9aa to
dd34ebe
Compare
ssun30
changed the title
ssun30
force-pushed
the
matlab_patches
branch
from
dd34ebe to
68f9bc8
Compare
|
Checklist for all unit tests (that can be implemented in MATLAB)
|
ssun30
force-pushed
the
matlab_patches
branch
from
83afc64 to
77a33c7
Compare
There was a problem hiding this comment.
Hi @ssun30 ... thanks for getting back to this! From my side, this looks mostly good to go (despite a minor nit that some newlines at file endings are missing). I did notice that the MATLAB CI run wasn't completed successfully, though.
Overall, it's great to have some of this covered, especially as the plan is to move towards replacing the current CLib with an autogenerated CLib (see Cantera/enhancements#220). Once this is in place, it will be relatively easy to expose parts that are currently missing from the MATLAB API (e.g. Reaction, Species, ReactionRate, SolutionArray, etc.). One thing that may also be necessary is a MATLAB equivalent to AnyMap to access the serialization interface - I believe that there's a new dictionary that was introduced in R2022b.
Previously the buffer length does not account for the null terminator, which causes the string returned from Clib to miss the last character. The correct error code is also implemented. The previous code will incorrectly throw an exception when the string is a single character.
into test class setup and teardown
for unit tests
for unit tests
to avoid calling the corresponding functions in clib and throw a warning
Added ImportPhases Method
ssun30
force-pushed
the
matlab_patches
branch
from
0ebe26f to
1228c04
Compare
Incompatibility Introduced by MATLAB's built-in Intel MKL library
Added transportModel to 'Water' PureFluid phase
ssun30
force-pushed
the
matlab_patches
branch
from
1228c04 to
6f778d9
Compare
They will warn the users whether to proceed since single-property setters can cause significant changes to other properties and multi-property setters are recommended.
The CI runner will crash when running this test class since the environment variables are not set properly. Revert this later.
ssun30
force-pushed
the
matlab_patches
branch
from
f78869f to
6c27a13
Compare
| @@ -1393,6 +1393,51 @@ function display(tp) | |||
|
|
|||
| end | |||
|
|
|||
| function set.T(tp, t) | |||
There was a problem hiding this comment.
I don't think that we should allow for single setter methods, and would in general avoid requiring input statements.
Changes proposed in this pull request
If applicable, fill in the issue number this pull request is fixing
Updates to Cantera/enhancements#177
If applicable, provide an example illustrating new features this pull request is introducing
Added tests for rates of progress, rate constants, species rates, and thermodynamic values.
Checklist
scons build&scons test) and unit tests address code coverage