Skip to content

Simplify Reactor evaluation/synchronization logic#2069

Merged
ischoegl merged 17 commits intoCantera:mainfrom
speth:reactor-eval-sync
Jan 28, 2026
Merged

Simplify Reactor evaluation/synchronization logic#2069
ischoegl merged 17 commits intoCantera:mainfrom
speth:reactor-eval-sync

Conversation

@speth
Copy link
Member

@speth speth commented Dec 21, 2025

Changes proposed in this pull request

This PR implements changes to how reactor governing equations are implemented to make use of the new requirement that all reactors have unique/independent contents.

  • Eliminate the syncState and restoreState methods of reactor objects. These steps are no longer necessary.
  • ReactorSurface objects are now responsible for calculating their own governing equations (for the surface coverages or moles). As a result, the getSurfaceInitialConditions and evalSurfaces methods are no longer necessary.
  • Introduce a new ReactorSurfaceFactory class for handling distinct reactor surface types. There are two versions of the newReactorSurface method, one which takes an explicit model type parameter, and another which creates the "right" type of ReactorSurface depending on the types of the adjacent Reactors (for example FlowReactorSurface for surfaces adjacent to a FlowReactor.
  • Added ExtensibleReactorSurface class to Python, which allows customization of the governing equations for surfaces.
  • Refactor process for assembling Jacobian elements. Individual reactors (and surfaces) now add elements to a vector<Triplet> where components indices are global indices within the network rather than reactor-local. This allows reactors to add terms for dependence on state variables of other reactors. Mostly supersedes Extensible jacobian and network jacobian interfaces #1634.
  • Should allow straightforward implementation of Create 'ReactorEdge' class in the reactor code to model edge interactions enhancements#203.

⚠️ No attempt has been made here to preserve backward compatibility. User code that makes use of ExtensibleReactor or otherwise creates derived types from the Reactor classes will likely need to be updated. Merging this PR commits us to a major version bump. ⚠️

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

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

A simple example of an ExtensibleReactorSurface (from test_reactor.py) now looks like this:

class CustomSurface(ct.ExtensibleReactorSurface):
    def replace_get_state(self, y):
        y[:] = 0
        y[kPts] = 1

    def replace_update_state(self, y):
        # this is the same thing the original method does
        self.phase.set_unnormalized_coverages(y)

        # Replace actual reactions with simple absorption of H2 -> H(S)
        C = self.reactors[0].phase.concentrations
        theta = self.phase.coverages
        self.rop = 1e-4 * C[kH2] * theta[kPts]
        self.net_production_rates[:] = 0.0
        self.net_production_rates[kH2_kin] = - self.rop

    def replace_eval(self, t, LHS, RHS):
        site_density = self.phase.site_density
        RHS[kHs] = 2 * self.rop / site_density
        RHS[kPts] = - 2 * self.rop / site_density

Previously, this required implementing an ExtensibleReactor type and using that to replace the rate calculations for the attached surface even though the bulk phase governing equations were not being modified.

AI Statement (required)

  • Limited use of generative AI. Standard or boilerplate code snippets were generated with AI and manually reviewed;
    all design, logic, and implementation decisions were made by the contributor.

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • AI Statement is included
  • The pull request is ready for review

@codecov
Copy link

codecov bot commented Dec 21, 2025

Codecov Report

❌ Patch coverage is 78.13051% with 124 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.17%. Comparing base (630ba99) to head (9fae77d).
⚠️ Report is 24 commits behind head on main.

Files with missing lines Patch % Lines
include/cantera/zeroD/ReactorBase.h 22.91% 37 Missing ⚠️
interfaces/cython/cantera/jacobians.pyx 23.07% 10 Missing ⚠️
src/zeroD/ReactorFactory.cpp 76.19% 9 Missing and 1 partial ⚠️
include/cantera/zeroD/ReactorDelegator.h 47.05% 9 Missing ⚠️
include/cantera/zeroD/ReactorSurface.h 61.90% 8 Missing ⚠️
interfaces/cython/cantera/reactor.pyx 82.97% 8 Missing ⚠️
src/zeroD/ReactorSurface.cpp 93.38% 4 Missing and 4 partials ⚠️
src/zeroD/ReactorNet.cpp 91.76% 0 Missing and 7 partials ⚠️
src/zeroD/IdealGasMoleReactor.cpp 71.42% 6 Missing ⚠️
src/zeroD/ConstPressureMoleReactor.cpp 54.54% 5 Missing ⚠️
... and 7 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2069      +/-   ##
==========================================
- Coverage   77.22%   77.17%   -0.05%     
==========================================
  Files         456      456              
  Lines       53026    52870     -156     
  Branches     8998     8951      -47     
==========================================
- Hits        40947    40805     -142     
- Misses       9035     9045      +10     
+ Partials     3044     3020      -24     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@speth speth force-pushed the reactor-eval-sync branch 3 times, most recently from 204d46f to b12fc94 Compare January 6, 2026 03:00
@speth speth marked this pull request as ready for review January 7, 2026 16:11
@speth speth requested a review from a team January 7, 2026 16:12
@speth speth force-pushed the reactor-eval-sync branch from b12fc94 to fa5cadf Compare January 8, 2026 21:05
ischoegl
ischoegl previously approved these changes Jan 25, 2026
Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @speth - sorry for not getting around to this sooner, and I apologize that I still don't have sufficient bandwidth for a full review.

With the caveat that we should bump the version to 4.0.0a1 prior to merging this PR, I am overall 👍 with the approach. I trust that our test suite is sufficiently thorough to catch regressions (caveat: I did notice some significant deviations of code in the flow reactor, where I don't think we have a comparison to numerical results in place).

@speth speth force-pushed the reactor-eval-sync branch from 971ed9e to 9fae77d Compare January 25, 2026 17:10
@ischoegl ischoegl mentioned this pull request Jan 27, 2026
6 tasks
@ischoegl ischoegl merged commit 86e4f33 into Cantera:main Jan 28, 2026
48 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve ReactorNet handling of bulk phase interactions Eliminate traps associated with accessing ThermoPhase objects in use by Reactors

2 participants