Preamble to InterfaceKinetics refactoring by ischoegl · Pull Request #1184 · Cantera/cantera

ischoegl · 2022-01-28T15:49:12Z

Changes proposed in this pull request

This PR seeks to address several issues that came up while working on #1181.

Fix compilation errors if NO_LEGACY_REACTIONS_26 is set
Streamline nomenclature for activationEnergy in the context of BlowersMasel ... use effectiveActivationEnergy for version that includes dependency on thermodynamic state (same will be introduced for ReactionRate classes that handle interface reactions).
Limit use of Arrhenius ... clarify as Arrhenius2 in legacy framework and make it equivalent with ArrheniusBase in new framework (ArrheniusBase should be renamed to Arrhenius after Cantera 2.6). Note: Defining a reduced Arrhenius(Base) class will also allow for additional templated classes - see e.g. Rethink handling of 3rd body colliders enhancements#133.
Remove leftover '3' from ChebyshevRate(3)
Calculate deltaH within BlowersMasel without requiring associated Kinetics object

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

Preamble to Refactor interface kinetics #1181
Closes Optionally disable legacy reactions in Cantera 2.6 enhancements#131

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

This PR will allow to disable legacy reactions (actually typedefs) from SCons as

$ scons build no_legacy_reactions=y

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
The pull request is ready for review

codecov · 2022-01-28T16:24:00Z

Codecov Report

Merging #1184 (ce94ac1) into main (2d772ab) will increase coverage by 0.05%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##             main    #1184      +/-   ##
==========================================
+ Coverage   65.39%   65.44%   +0.05%     
==========================================
  Files         318      318              
  Lines       46109    46106       -3     
  Branches    19601    19604       +3     
==========================================
+ Hits        30153    30175      +22     
+ Misses      13452    13435      -17     
+ Partials     2504     2496       -8

Impacted Files	Coverage Δ
include/cantera/kinetics/FalloffFactory.h	`90.00% <ø> (ø)`
include/cantera/kinetics/FalloffMgr.h	`92.85% <ø> (ø)`
include/cantera/kinetics/GasKinetics.h	`100.00% <ø> (ø)`
include/cantera/kinetics/MultiRateBase.h	`100.00% <ø> (ø)`
include/cantera/kinetics/RateCoeffMgr.h	`100.00% <ø> (ø)`
include/cantera/kinetics/Reaction.h	`100.00% <ø> (ø)`
include/cantera/kinetics/ReactionData.h	`93.75% <0.00%> (+1.91%)`	⬆️
src/kinetics/FalloffFactory.cpp	`92.85% <ø> (ø)`
src/kinetics/GasKinetics.cpp	`88.99% <50.00%> (ø)`
include/cantera/kinetics/Falloff.h	`77.98% <61.53%> (ø)`
... and 18 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 2d772ab...ce94ac1. Read the comment docs.

ischoegl · 2022-01-30T16:42:49Z

@speth ... this should take care of a lot of accumulated lint, and clearly mark legacy objects wherever used. Efficient handling of the BlowersMasel API will be important for #1181, where some preparatory work is done here. The new CI runner ensures that the typedef Arrhenius2 Arrhenius can be safely disabled.

There should be only minimal overlap with #1183 (if any). I added one instance of setContext, which I believe is a workable solution to the removal of remaining Reaction specializations that involve third bodies.

Co-authored-by: Ray Speth <[email protected]>

ChebyshevRate3 was introduced to avoid a conflict with the pre-existing class 'ChebyshevRate'. Since its introduction, the conflict was removed. This commit restores the original name to this class.

Co-authored-by: Ray Speth <[email protected]>

ischoegl · 2022-02-04T21:33:27Z

@speth ... rebased now that #1183 is merged. Once this PR is resolved, I can get back to InterfaceKinetics (#1181).

speth

Thanks for splitting this out, since a lot of this is just reorganization and name changes. I think it will make reviewing #1181 a fair bit easier.

I like the addition of setContext -- it makes a lot of sense to have a function that can be called to set things like species indices that can be resolved at the point when a reaction is added to a Kinetics object, but not before.

include/cantera/kinetics/RxnRates.h

include/cantera/kinetics/Falloff.h

include/cantera/kinetics/FalloffMgr.h

include/cantera/kinetics/Arrhenius.h

interfaces/cython/cantera/reaction.pyx

test/kinetics/kineticsFromScratch3.cpp

include/cantera/kinetics/Arrhenius.h

ischoegl · 2022-02-05T23:58:49Z

@speth ... I addressed all comments. GH actions appears down for the time being; I'll try to re-trigger later on. Regarding the renaming of ArrheniusBase, I am tabling this as there is another tweak that I'd like to test first.

ischoegl · 2022-02-06T02:01:09Z

GH actions are back up, and everything ran through …

include/cantera/kinetics/Arrhenius.h

- Add todo item for deprecation of Arrhenius2 constructor - Add missing override declarations in BlowersMaselRate

speth

Thanks for the updates, @ischoegl. This looks good to me.

ischoegl marked this pull request as draft January 28, 2022 15:49

ischoegl force-pushed the interface-refactor-preamble branch from 3fe51f0 to 04629b0 Compare January 28, 2022 15:57

ischoegl mentioned this pull request Jan 28, 2022

Optionally disable legacy reactions in Cantera 2.6 Cantera/enhancements#131

Closed

ischoegl force-pushed the interface-refactor-preamble branch 7 times, most recently from 847a3a5 to a1c4b5c Compare January 30, 2022 03:38

ischoegl mentioned this pull request Jan 30, 2022

Refactor Reaction Rate Evaluators Cantera/enhancements#87

Closed

ischoegl force-pushed the interface-refactor-preamble branch 3 times, most recently from 906afe7 to f5008ce Compare January 30, 2022 16:37

ischoegl marked this pull request as ready for review January 30, 2022 16:37

ischoegl requested a review from speth January 30, 2022 16:38

This was referenced Jan 31, 2022

Eliminate unnecessary specialized Reaction types #1183

Merged

Rethink handling of 3rd body colliders Cantera/enhancements#133

Closed

ischoegl and others added 7 commits February 4, 2022 13:39

[SCons] Add option to disable legacy reactions

9758427

[CI] Add runner probing 'no_legacy_reactions' option

ef61299

[Kinetics] Fix compilation errors with 'no_legacy_reactions'=y

7a45e71

[Kinetics] Make ArrheniusBase independent from ReactionRate

a0359c4

[Kinetics] (Re)-implement updateFromStruct

1cd0343

Co-authored-by: Ray Speth <[email protected]>

[Kinetics] Rename MultiBulkRate to MultiRate

13b2986

[Kinetics] Add ReactionRate::setContext

185ced9

ischoegl force-pushed the interface-refactor-preamble branch from f5008ce to 74650d2 Compare February 4, 2022 20:01

ischoegl added 2 commits February 4, 2022 15:18

[Python] Consolidate constructors and properties in reaction.pyx

6ac4c46

Remove remmants of BlowersMasel2

df0082a

ischoegl and others added 7 commits February 4, 2022 15:18

Switch PlogRate to use ArrheniusBase and limit use of 'Arrhenius'

94e471b

Mark remaining legacy objects with '2' suffix

9f7b5ad

[Kinetics] Add deprecation messages to legacy files

4179a5c

Remove suffix '3' from ChebyshevRate(3)

c31f8eb

ChebyshevRate3 was introduced to avoid a conflict with the pre-existing class 'ChebyshevRate'. Since its introduction, the conflict was removed. This commit restores the original name to this class.

[Kinetics] Calculate Blowers-Masel deltaH internally

1165820

[Kinetics] Re-introduce updateFromStruct instances

712d239

Co-authored-by: Ray Speth <[email protected]>

[Kinetics] Remove remaining 'bulk' references and add todo comments

7a6f1bd

ischoegl force-pushed the interface-refactor-preamble branch from 74650d2 to 7a6f1bd Compare February 4, 2022 21:18

speth requested changes Feb 5, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

ischoegl requested a review from speth February 5, 2022 21:02

ischoegl force-pushed the interface-refactor-preamble branch from 63e2f88 to 440a563 Compare February 5, 2022 22:07

ischoegl added 2 commits February 5, 2022 17:47

Various code and docstring improvements

e50d243

Add ArrheniusTypeRate to Sphinx documentation

5fa23b7

ischoegl marked this pull request as draft February 5, 2022 23:50

ischoegl force-pushed the interface-refactor-preamble branch from 2709a41 to 5fa23b7 Compare February 5, 2022 23:52

ischoegl marked this pull request as ready for review February 5, 2022 23:56

Fix typo in 'SConstruct'

1ef59d5

ischoegl force-pushed the interface-refactor-preamble branch from 7dbc17d to 3c0c53d Compare February 6, 2022 17:07

speth reviewed Feb 6, 2022

View reviewed changes

include/cantera/kinetics/Arrhenius.h Show resolved Hide resolved

Minor updates

ce94ac1

- Add todo item for deprecation of Arrhenius2 constructor - Add missing override declarations in BlowersMaselRate

ischoegl force-pushed the interface-refactor-preamble branch from 3c0c53d to ce94ac1 Compare February 6, 2022 18:17

speth approved these changes Feb 6, 2022

View reviewed changes

speth merged commit f85d54f into Cantera:main Feb 6, 2022

ischoegl mentioned this pull request Feb 7, 2022

Introduce BulkRate class template #1187

Merged

5 tasks

ischoegl deleted the interface-refactor-preamble branch February 7, 2022 02:22

ischoegl mentioned this pull request Feb 22, 2022

Plasma phase #1190

Merged

5 tasks

Uh oh!

Conversation

ischoegl commented Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ischoegl commented Jan 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ischoegl commented Feb 4, 2022

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!

This comment was marked as outdated.

ischoegl commented Feb 5, 2022

Uh oh!

ischoegl commented Feb 6, 2022

Uh oh!

Uh oh!

speth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ischoegl commented Jan 28, 2022 •

edited

Loading

codecov bot commented Jan 28, 2022 •

edited

Loading

ischoegl commented Jan 30, 2022 •

edited

Loading