Delegating reactor by chinahg · Pull Request #1130 · Cantera/cantera

chinahg · 2021-10-23T22:59:32Z

Changes proposed in this pull request

Added capability to modify governing equations in Cantera reactors in python

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

Closes #

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

Checklist

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

…plementations

For 'after' delegates, if the return code from the wrapped function is non-zero, then the delegate's return value will be the sum of the values from the base and wrapped functions.

Avoids name clashes with existing Python names that may have different method signatures, e.g. Reactor.get_state.

The function for evaluating dy/dt no longer needs the vector of sensitivity parameters, and should not operate directly on the state vector (which should be processed by updateState).

…tors

codecov · 2021-10-29T18:31:42Z

Codecov Report

Merging #1130 (95f3c21) into main (4570f3e) will increase coverage by 0.14%.
The diff coverage is 83.19%.

❗ Current head 95f3c21 differs from pull request most recent head ff6f6a8. Consider uploading reports for the commit ff6f6a8 to get more accurate results

@@            Coverage Diff             @@
##             main    #1130      +/-   ##
==========================================
+ Coverage   73.49%   73.64%   +0.14%     
==========================================
  Files         365      372       +7     
  Lines       48187    49257    +1070     
==========================================
+ Hits        35417    36273     +856     
- Misses      12770    12984     +214

Impacted Files	Coverage Δ
include/cantera/base/AnyMap.h	`100.00% <ø> (ø)`
include/cantera/base/Solution.h	`100.00% <ø> (ø)`
include/cantera/base/YamlWriter.h	`100.00% <ø> (ø)`
include/cantera/base/global.h	`93.33% <ø> (ø)`
include/cantera/base/utilities.h	`100.00% <ø> (ø)`
include/cantera/base/xml.h	`100.00% <ø> (ø)`
include/cantera/equil/MultiPhase.h	`100.00% <ø> (ø)`
include/cantera/kinetics/FalloffFactory.h	`100.00% <ø> (ø)`
include/cantera/kinetics/GasKinetics.h	`100.00% <ø> (ø)`
include/cantera/kinetics/Kinetics.h	`55.26% <0.00%> (-1.50%)`	⬇️
... and 157 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4570f3e...ff6f6a8. Read the comment docs.

speth

Thanks, @chinahg for opening this PR. I think most of the comments I have were about formatting issues. I see in the commit history a bit of "churn" where certain changes are made and then mostly undone in subsequent commits. I hope you can do a little tidying-up of these so that each commit stands alone as a complete and functional idea.

speth · 2021-11-18T14:42:09Z

@chinahg - One additional thing -- the tests which run the Python examples are failing because interfaces/cython/cantera/examples/reactors/custom2.py needs to be updated to use the new LHS/RHS version of eval.

Implemented LHS/RHS notation in eval functions of Reactor classes Updated ReactorDelegator for new eval arguments

Required for LHS/RHS arguments Added name to authors list

Whitespace deletions, unnecessary comment removal, Doxygen string formatting

speth · 2021-12-17T00:21:29Z

The additional commits here have now been incorporated back into #1003, so I'm going to close this PR.

ischoegl mentioned this pull request Oct 24, 2021

Delegate reactor implementation to user-provided Python classes #1003

Merged

10 tasks

speth added 20 commits October 29, 2021 12:35

[Reactor] Proof of concept for delegating methods to Python

486d726

[Reactor] Make reference counting for delegated methods more robust

f4ba197

[Reactor] Add ability to delegate functions that take array arguments

fc33a19

[Reactor] Generalize mechanism for registering delegated functions

387aee4

[Reactor] Add ability to delegate methods with return values

98b5801

[Reactor] De-virtualize some generic get/set functions

0d1790a

[Reactor] Add delegates for remaining core Reactor functions

935fcfc

[Reactor] Make some member variables available to DelegatedReactor im…

c910a95

…plementations

[Reactor] Added a DelegatedReactor example with an extra equation

b574115

Modify delegate creator functions to return the delegate

1fdd9c7

Generalize interface for delegator classes

2d8d39c

Revise behavior of "after" delegates that return a value

ce2d309

For 'after' delegates, if the return code from the wrapped function is non-zero, then the delegate's return value will be the sum of the values from the base and wrapped functions.

[Python] Prefix replacement delegate methods with 'replace_'

c7c76a2

Avoids name clashes with existing Python names that may have different method signatures, e.g. Reactor.get_state.

[Reactor] Use ReactorNet to manage application of sensitivity params

8d15e6b

[Reactor] Introduce simpler function for reactor ODE function

d44a696

The function for evaluating dy/dt no longer needs the vector of sensitivity parameters, and should not operate directly on the state vector (which should be processed by updateState).

[Python] Add some basic tests of delegated reactors

fd536eb

[Reactor] Add methods for restoring thermo states from delegated reac…

21a15c2

…tors

[Python] Support multiple levels of inheritance in delegated classes

602cebc

[Python] Improve syntax for specifying signatures of delegated methods

a429ddc

[Doc] Add documentation for delegation capabilities

11c6093

chinahg force-pushed the delegating-reactor branch from af59e7d to 3df450c Compare October 29, 2021 17:54

speth requested changes Nov 18, 2021

View reviewed changes

chinahg force-pushed the delegating-reactor branch from 3df450c to 7dcaea8 Compare December 6, 2021 19:54

chinahg added 4 commits December 6, 2021 18:08

Added RHS and LHS user variables to Reactor class

0f12c61

Implemented LHS/RHS notation in eval functions of Reactor classes Updated ReactorDelegator for new eval arguments

Created new setDelegate method for void(double, double*, double*) type

1970afe

Required for LHS/RHS arguments Added name to authors list

Passes test suite, includes new test for RHS/LHS implementation

2981339

Minor formatting changes

51d432e

Whitespace deletions, unnecessary comment removal, Doxygen string formatting

Added custom2.py updated eval fxn

ff6f6a8

chinahg force-pushed the delegating-reactor branch from 7dcaea8 to ff6f6a8 Compare December 6, 2021 23:15

speth closed this Dec 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Delegating reactor#1130

Delegating reactor#1130
chinahg wants to merge 25 commits into
Cantera:mainfrom
chinahg:delegating-reactor

chinahg commented Oct 23, 2021

Uh oh!

codecov Bot commented Oct 29, 2021 •

edited

Loading

Uh oh!

speth left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

speth commented Nov 18, 2021

Uh oh!

speth commented Dec 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chinahg commented Oct 23, 2021

Uh oh!

codecov Bot commented Oct 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

speth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

speth commented Nov 18, 2021

Uh oh!

speth commented Dec 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Oct 29, 2021 •

edited

Loading