Skip to content

Deprecate redundant kinetics methods in Python API#1202

Merged
ischoegl merged 8 commits intoCantera:mainfrom
ischoegl:deprecate-redundant-kinetics-methods
Mar 5, 2022
Merged

Deprecate redundant kinetics methods in Python API#1202
ischoegl merged 8 commits intoCantera:mainfrom
ischoegl:deprecate-redundant-kinetics-methods

Conversation

@ischoegl
Copy link
Copy Markdown
Member

@ischoegl ischoegl commented Feb 20, 2022 •
edited
Loading

Changes proposed in this pull request

  • Deprecate redundant / unpythonic methods that mimic the C-API (all alternative methods already exist):
    • Kinetics.reaction_type(i) -> Kinetics.reaction(i).reaction_type
    • Kinetics.is_reversible(i) -> Kinetics.reaction(i).reversible
    • Kinetics.reaction_equation(i) -> Kinetics.reaction(i).equation
    • Kinetics.reactants(i) -> Kinetics.reaction(i).reactant_string
    • Kinetics.products(i) -> Kinetics.reaction(i).product_string
  • Improve rendering of Reaction.__repr__, where essential information was lost in Eliminate unnecessary specialized Reaction types #1183
  • Fix indentation and blank lines for Sphinx docstring deprecation information

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

After #1183, several reactions no longer show specialized information (which is held by the ReactionRate), where specifically no identifiable information is shown for Chebyshev and pressure-dependent-Arrhenius (Plog) (as well as the new Blowers-Masel and two-temperature-plasma types), e.g.

in [1]: import cantera as ct
   ...: gas = ct.Solution("test/data/kineticsfromscratch.yaml")
   ...: gas.reactions()
Out[1]: 
[<Reaction: H2 + O <=> H + OH>,
 <ThreeBodyReaction: 2 O + M <=> O2 + M>,
 <FalloffReaction: 2 OH (+M) <=> H2O2 (+M)>,
 <Reaction: H2 + O2 <=> 2 OH>,
 <Reaction: HO2 <=> O + OH>,
 <ThreeBodyReaction: H + O2 + O2 <=> HO2 + O2>,
 <Reaction: H2 + O <=> H + OH>,
 <FalloffReaction: 2 OH (+M) <=> H2O2 (+M)>,
 <FalloffReaction: H + HO2 (+M) <=> H2 + O2 (+M)>,
 <FalloffReaction: H + HO2 (+M) <=> H2 + O2 (+M)>,
 <ChemicallyActivatedReaction: H2O + OH (+M) <=> H2 + HO2 (+M)>,
 <Reaction: H + O => H + O>,
 <Reaction: O + OH => O + OH>]

This PR reformats Reaction.__repr__ to always include information for the rate specialization.

In [1]: import cantera as ct
   ...: gas = ct.Solution("test/data/kineticsfromscratch.yaml")
   ...: gas.reactions()
Out[1]: 
[H2 + O <=> H + OH    <Reaction(Arrhenius)>,
 2 O + M <=> O2 + M    <ThreeBodyReaction(Arrhenius)>,
 2 OH (+M) <=> H2O2 (+M)    <FalloffReaction(Troe)>,
 H2 + O2 <=> 2 OH    <Reaction(pressure-dependent-Arrhenius)>,
 HO2 <=> O + OH    <Reaction(Chebyshev)>,
 H + O2 + O2 <=> HO2 + O2    <ThreeBodyReaction(Arrhenius)>,
 H2 + O <=> H + OH    <Reaction(Blowers-Masel)>,
 2 OH (+M) <=> H2O2 (+M)    <FalloffReaction(Lindemann)>,
 H + HO2 (+M) <=> H2 + O2 (+M)    <FalloffReaction(SRI)>,
 H + HO2 (+M) <=> H2 + O2 (+M)    <FalloffReaction(Tsang)>,
 H2O + OH (+M) <=> H2 + HO2 (+M)    <ChemicallyActivatedReaction(Lindemann)>,
 H + O => H + O    <Reaction(two-temperature-plasma)>,
 O + OH => O + OH    <Reaction(two-temperature-plasma)>]

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

@ischoegl ischoegl force-pushed the deprecate-redundant-kinetics-methods branch from 1f0098c to 8ce1924 Compare February 20, 2022 15:24
@ischoegl ischoegl changed the title Deprecate redundant kinetics methods Deprecate redundant kinetics methods in Python API Feb 20, 2022
@ischoegl ischoegl force-pushed the deprecate-redundant-kinetics-methods branch from 8ce1924 to ab9c4ab Compare February 20, 2022 16:04
@ischoegl
[Python] Remove Kinetics.reaction_type_str
e7ced77 
The method (introduced in f0868c7) is not part of a stable release and holds
was intended to facilitate transitional behavior of the Kinetics.reaction_type
method. Instead, the Kinetics.reaction_type method is completely replaced by the
ReactionRate.type property of a specific Reaction.
@ischoegl
[Python] Mark redundant Kinetics methods as deprecated
1589896 
Deprecated methods follow the C-API style for reaction-specific information
and should be accessed from the associated Reaction object instead.
@ischoegl
[Python] Add missing blank lines and fix indentation for Sphinx docst…
03904df 
…rings
@ischoegl
[Samples] Replace deprecated method
ecb8aad
@ischoegl
[UnitTests] Replace deprecated methods
7a7769b
@ischoegl
[Python] Improve Reaction.__repr__ for new framework
ed1571e
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 20, 2022 •
edited
Loading

Codecov Report

Merging #1202 (0889b09) into main (041559d) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1202      +/-   ##
==========================================
- Coverage   65.41%   65.38%   -0.03%     
==========================================
  Files         318      318              
  Lines       46085    46095      +10     
  Branches    19604    19604              
==========================================
- Hits        30145    30139       -6     
- Misses      13426    13442      +16     
  Partials     2514     2514
Impacted Files Coverage Δ
src/base/application.h 69.76% <0.00%> (-15.95%) ⬇️
src/base/global.cpp 70.58% <0.00%> (-4.26%) ⬇️
include/cantera/base/global.h 81.81% <0.00%> (-2.40%) ⬇️
src/base/application.cpp 63.98% <0.00%> (-1.26%) ⬇️
src/kinetics/Kinetics.cpp 72.47% <0.00%> (-0.46%) ⬇️
include/cantera/cython/wrappers.h 85.15% <0.00%> (-0.33%) ⬇️
src/kinetics/Reaction.cpp 81.07% <0.00%> (+0.17%) ⬆️
src/kinetics/Falloff.cpp 85.39% <0.00%> (+0.74%) ⬆️
include/cantera/base/logger.h 78.57% <0.00%> (+5.84%) ⬆️

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 041559d...0889b09. Read the comment docs.

@ischoegl ischoegl force-pushed the deprecate-redundant-kinetics-methods branch from ab9c4ab to ed1571e Compare February 20, 2022 16:36
@ischoegl ischoegl marked this pull request as ready for review February 20, 2022 16:39
@ischoegl ischoegl requested a review from bryanwweber February 20, 2022 17:02
speth
speth previously approved these changes Mar 5, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

Looks good to me -- I just had one minor suggestion, but I don't think it affects much either way.

Longer term, I'd like to eliminate these from C++ as well, though that would require updating the C / Matlab / Fortran APIs, which currently have no notion of a Reaction object.

bryanwweber
bryanwweber requested changes Mar 5, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

Thanks @ischoegl! A few small comments.

@ischoegl ischoegl force-pushed the deprecate-redundant-kinetics-methods branch from b9e3e56 to 406ef98 Compare March 5, 2022 16:54
@ischoegl ischoegl requested a review from bryanwweber March 5, 2022 16:54
@ischoegl ischoegl force-pushed the deprecate-redundant-kinetics-methods branch from 406ef98 to 0889b09 Compare March 5, 2022 17:08
bryanwweber
bryanwweber approved these changes Mar 5, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

Thanks @ischoegl looks good to me! I'll wait for @speth to approve as well, since he also left a review.

@bryanwweber
Copy link
Copy Markdown
Member

bryanwweber commented Mar 5, 2022

Ah, I see @speth approved. Feel free to :shipit: once the checks are all green.

@ischoegl ischoegl merged commit 551a45c into Cantera:main Mar 5, 2022
@ischoegl ischoegl deleted the deprecate-redundant-kinetics-methods branch March 5, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bryanwweber bryanwweber bryanwweber approved these changes

@speth speth speth left review comments

Assignees

No one assigned

Labels

Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ischoegl @bryanwweber @speth