Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing array properties to SolutionArray and FlameBase #1342

Merged
merged 4 commits into from
Jul 29, 2022

Conversation

speth
Copy link
Member

@speth speth commented Jul 27, 2022

Changes proposed in this pull request

  • Add array properties that were missing from the FlameBase and SolutionArray classes. Most of these are the derivatives of the reaction rates / rates of progress, but there were a few others as well, such as FlameBase.mean_molecular_weight
  • Add unit tests that check that all propertys of the Solution object are extended to the corresponding array form in the FlameBase and Solution classes, except for those that are specifically excluded. This will prevent new properties of the Solution class from being neglected in the future.
  • Add the equivalence_ratio property to CounterflowDiffusionFlame

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

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

Extend test suite to check that all properties of the Solution class
either return appropriate arrays in the FlameBase class or are
specifically excluded.
@speth speth added the Python label Jul 27, 2022
Extend test suite to check that all properties of the Solution class
either return appropriate arrays in the SolutionArray class or are
specifically excluded.
@codecov
Copy link

codecov bot commented Jul 27, 2022

Codecov Report

Merging #1342 (89835c5) into main (3a294dd) will increase coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1342      +/-   ##
==========================================
+ Coverage   68.03%   68.15%   +0.11%     
==========================================
  Files         327      327              
  Lines       42652    42658       +6     
  Branches    17180    17180              
==========================================
+ Hits        29020    29074      +54     
+ Misses      11345    11291      -54     
- Partials     2287     2293       +6     
Impacted Files Coverage Δ
src/oneD/Sim1D.cpp 74.01% <0.00%> (-0.27%) ⬇️
include/cantera/thermo/ThermoPhase.h 44.58% <0.00%> (+3.18%) ⬆️
include/cantera/cython/kinetics_utils.h 91.42% <0.00%> (+8.57%) ⬆️
src/kinetics/Kinetics.cpp 79.11% <0.00%> (+9.78%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

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 - changes look good to me.

@ischoegl ischoegl merged commit ade1a38 into Cantera:main Jul 29, 2022
@speth speth deleted the array-properties branch July 29, 2022 13:59
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.

2 participants