Various minor fixes to the Matlab toolbox - #2168
Merged
Merged
Conversation
Resolves the Code Analyzer warning about disp(sprintf(...)) in the MATLAB samples. The four sites in flamespeed.m already ended their format strings with a newline; rankine.m gains one so its output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The only behavior this overload needs to define is the function-call form
f(x). The '.' branch and the leading length(s) > 1 chaining block were
hand-rolled reimplementations of default subsref behavior, and worse at it
than the default: the chaining block assumed s(1).type was '.' without
checking, so f(2).type failed on a cell-array field name, and the
fall-through error ('Specify value for x as p(x)') was misleading for a
brace reference.
Handle '()' and pass everything else to builtin('subsref', obj, s), with a
comment saying why, and trim the docstring to document the interesting case
instead of restating default behavior.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The functor-type bullet list in the constructor docstring was indented past the surrounding text, nesting it in a blockquote, and its usage examples lacked the '::' marker, so they rendered as running text with smart quotes instead of literal blocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ct.oneD.Flow is the base class for the flow domains but was missing from the Sphinx listing, so its properties and methods were undocumented and the concrete flow domains showed no base class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
auto_path.glob() returns a generator that was fully consumed by the loop building the list of generated files, so env.Depends() was handed an empty list and edits to interfaces/sourcegen/.../headers/ct*.yaml never triggered regeneration of the CLib sources. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The method passed an extra trailing argument to surf_setCoverages, which does not match the generated CLib signature and would not have reached SurfPhase::setCoveragesNoNorm in any case. Add a CLib recipe for setCoveragesNoNorm and call it, with tests on both sides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CLib provided only the name-based overload, so the index-based form was unreachable from MATLAB and Julia. Following the sol_adjacent / sol_adjacentByName precedent, reactornet_sensitivity now wraps sensitivity(size_t, size_t) and the name-keyed form becomes reactornet_sensitivityByName. MATLAB's ReactorNet.sensitivity now dispatches on the type of the component argument, errors on an unsupported type, and passes a reactor index rather than a cabinet handle; it accepts a reactor object or a 1-based position, and ReactorNet gains a reactors property to support the lookup. Julia gains the index-based method. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The toolbox provided MassFlowController and Valve but no PressureController. Two bugs blocked it: FlowDevice.setPrimary passed the device object where CLib expects a handle, and the only coefficient setter refused anything that was not a Valve. Add a generic deviceCoefficient property that valveCoeff and the new pressureCoeff both delegate to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The method was declared with an output argument that its body never assigned, so any caller asking for one got "Output argument 'tp' not assigned" rather than the copy the signature implied. equilibrate modifies the phase in place; declare it with no output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The method returned the CLib status code and documented it as "The error in the solution". It is neither: MultiPhase::equilibrate returns void, mix_equilibrate returns 0 on success and -1 on failure, and ct.impl.call already turns the failure code into a Cantera:ctError. The value handed back was therefore an unconditional zero described as a solver residual. The claim predates the toolbox rewrite -- the legacy MEX interface removed in 5e275fc carried the same docstring over the same always-zero return. Declare the method with no output, matching ThermoPhase.equilibrate and Python's Mixture.equilibrate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get.massFlowRate declared its output argument as "flag" but assigned the CLib result to a local "rate", so the output was never set and every read of the property raised MATLAB:unassignedOutputs. set.massFlowRate also assigned obj.massFlowRate at the end. That is not recursive -- MATLAB suppresses a property's set method for an assignment made inside that same method -- but it is dead: get.massFlowRate shadows the stored value, so nothing could ever read it. Drop it, matching the forward-only set methods in ReactorBase. Expand the property documentation to describe both directions, following FlowReactor::massFlowRate (m_u * m_rho * m_area) and setMassFlowRate, which sets the speed from the current density and area. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The docstring for the coverageEnabled property was copied from an unrelated method: it described "setting bounds on the solution components" and documented a `:param flag:` for a call form rather than describing the property itself. Replace it with a description of what the flag actually controls, matching the Python `coverage_enabled` docstring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The default steady-state tolerances and maximum Jacobian age are good enough for these examples, so the explicit setSteadyTolerances, setTransientTolerances and setMaxJacAge calls were just noise for readers. The equivalent Python examples set none of them. diffusion_flame is the exception: it still needs a relaxed transient absolute tolerance, since the default is too tight for the time stepping used to get the flame started and the solver stalls at its minimum timestep. Only that one call is kept, with the relative tolerance left at its default and a comment saying why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `concentrations` property was reported broken in the experimental toolbox. It works on main: the getter read the object handle from an undefined `tp` between 4d0aa3b and f5811df, a window that only the v3.2.0b1 tag ever shipped. Cover both the bulk and the surface phase so the regression cannot come back unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The docstring promised a cell array of directories, but the function returned the single PATH-like string that CLib builds, and it hard-coded ';' as the separator handed to Cantera::getDataDirectories -- wrong on every platform where pathsep is ':'. Pass pathsep in both directions and split the result, so the join and the split cannot drift apart. This matches get_data_directories() in the Python module, which uses os.pathsep the same way. Add ctTestUtility, a home for the package-level ct.* utility functions, covering the shape of the returned cell array. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2168 +/- ##
==========================================
+ Coverage 78.20% 78.29% +0.08%
==========================================
Files 453 454 +1
Lines 55448 55478 +30
Branches 9121 9122 +1
==========================================
+ Hits 43364 43437 +73
+ Misses 9036 8994 -42
+ Partials 3048 3047 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Neither ct.zeroD.ReactorBase nor ct.zeroD.ReactorNet offered any way to map between a state-vector index and a component name, so MATLAB code needing a global index had to hard-code the state-vector layout of the reactor type it was using. Add four CLib recipes -- reactor_neq, reactor_componentName, reactor_componentIndex and reactornet_globalComponentIndex -- and the six corresponding MATLAB members: ReactorBase.nVars, ReactorBase.componentIndex, ReactorBase.componentName, ReactorNet.nVars, ReactorNet.componentName and ReactorNet.globalComponentIndex. All MATLAB indices are 1-based in both directions. globalComponentIndex takes its reactor argument in the same forms as ReactorNet.sensitivity, and that dispatch is now a shared private helper that also range-checks a numeric index; without it, an out-of-range index reaches an unchecked m_reactors[reactor] in C++. Note that ReactorNet.componentName returns a reactor-prefixed name such as 'reactor1: temperature', while globalComponentIndex takes the unprefixed name resolved within a given reactor; the two are not inverses. The sensitivity tests added alongside the ReactorNet.sensitivity work now take their global indices from globalComponentIndex rather than from the literals 3 and 3 + nSpecies. No C++ change is required. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`clib_defs.h` defines four values that a generated CLib function can return to signal that an exception was raised: -1 and -2 (for functions whose normal return value is a length or a handle), `DERR` (-999.999), and `ERR` (-999). `ct.impl.errorCode` listed every one of these but `ERR`, so a function generated as a "size getter" -- `domain_componentIndex`, `reactor_neq`, `reactornet_globalComponentIndex` and the like -- returned -999 to MATLAB as if it were a valid result, and the exception was silently swallowed. Add -999 to the list, and drop the two local workarounds in `ct.zeroD.ReactorBase.componentIndex` and `ct.zeroD.ReactorNet.globalComponentIndex` that this makes dead. The Cantera error message is unchanged; it now comes from `ct.impl.call` instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
speth
marked this pull request as ready for review
August 28, 2026 18:58
ischoegl
approved these changes
Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes proposed in this pull request
Matlab-specific updates:
disp(sprintf(...))instead of directly callingfprintfFunc1.subsrefand give it a docstring that makes senseFunc1lists and code blocksFlowclass in the API documentationInterface.setUnnormalizedCoverageswhich was incorrectly callingsurf_setCoverageswith an extra, non-existing argumentPressureControllerclassThermoPhase.equilibrateandMixture.equilibrateFlowReactor.massFlowRate, which previously failed while trying to return an undefined variableReactingSurface.coverageEnabledThermoPhase.concentrationsct.dataDirectoriesto return a cell array as indicated in the docstring (and for consistency with Python'sct.get_data_directories()methodReactorandReactorNetfor getting component names and indices-999is the sentinel error value returned by CLibOther fixes:
ReactorNet::sensitivity, with the former now namedreactornet_sensitivityByName, analogous tosol_adjacentByNameinterfaces/sourcegen/.../headers/ct*.yamltrigger rebuilds of CLibAI Statement (required)
Checklist
scons build&scons test) and unit tests address code coverage