Refactor StoichManager for sparse Jacobian Calculations by ischoegl · Pull Request #1081 · Cantera/cantera

ischoegl · 2021-08-01T00:38:44Z

Changes proposed in this pull request

Simplify StoichManager
Leverage <Eigen/Sparse> to handle sparse multiplications
Implement calculation of sparse Jacobian terms

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

See #Cantera/enhancements#111 and #Cantera/enhancements#100

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

While calculations are handled by <Eigen/Sparse> internally, results are exposed to the Python API as:

>>> gas.net_rop_species_derivatives # similar for forward/reverse
>>> gas.net_rop_temperature_derivatives # similar for forward/reverse
>>> gas.net_production_rate_species_derivatives # similar for creation/destruction
>>> gas.net_production_rate_temperature_derivatives # similar for creation/destruction

All derivatives are checked against numerical implementations in unit tests. ~~At the moment, third-body terms are not considered.~~ (Edit: now done.) There may be some overlap with #1010, where Jacobians are evaluated for entire ReactorNets (whereas this PR focuses on GasKinetics).

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 · 2021-08-01T01:10:21Z

Codecov Report

Merging #1081 (e1c9286) into main (e77148b) will increase coverage by 0.03%.
The diff coverage is 76.41%.

❗ Current head e1c9286 differs from pull request most recent head 53b764e. Consider uploading reports for the commit 53b764e to get more accurate results

@@            Coverage Diff             @@
##             main    #1081      +/-   ##
==========================================
+ Coverage   73.45%   73.48%   +0.03%     
==========================================
  Files         364      362       -2     
  Lines       47611    48009     +398     
==========================================
+ Hits        34972    35280     +308     
- Misses      12639    12729      +90

Impacted Files	Coverage Δ
include/cantera/base/global.h	`93.33% <ø> (ø)`
include/cantera/kinetics/Kinetics.h	`51.35% <0.00%> (-4.54%)`	⬇️
src/clib/ct.cpp	`8.29% <0.00%> (ø)`
src/numerics/DenseMatrix.cpp	`62.93% <ø> (ø)`
src/numerics/polyfit.cpp	`94.44% <ø> (ø)`
include/cantera/kinetics/ReactionRate.h	`65.43% <21.42%> (+4.09%)`	⬆️
src/kinetics/GasKinetics.cpp	`79.62% <64.86%> (-12.99%)`	⬇️
src/kinetics/Kinetics.cpp	`79.75% <73.68%> (-4.18%)`	⬇️
include/cantera/cython/wrappers.h	`81.81% <77.77%> (-1.79%)`	⬇️
include/cantera/kinetics/StoichManager.h	`96.73% <94.69%> (-1.91%)`	⬇️
... and 27 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 e77148b...53b764e. Read the comment docs.

bryanwweber · 2021-08-02T00:18:34Z

I think to fix the failures on the Conda builds, you need to include config.h before you check whether CT_USE_SYSTEM_EIGEN is true. At least, that's what's done in eigen_dense.h, via ct_defs.h.

ischoegl · 2021-08-02T03:37:02Z

I think to fix the failures on the Conda builds, you need to include config.h before you check whether CT_USE_SYSTEM_EIGEN is true. At least, that's what's done in eigen_dense.h, via ct_defs.h.

👍 - Ah - I was wondering about that. Thanks! I ended up moving the include of StoichManager.h to the *.cpp files, which appears to have solved the issue by a different route. Main reason was that it didn't need to be included everywhere ...

bryanwweber · 2021-08-02T13:42:34Z

I ended up moving the include of StoichManager.h to the *.cpp files, which appears to have solved the issue by a different route.

👍 Nonetheless, I still think it's a good idea to make sure that config.h is included before checking CT_USE_SYSTEM_EIGEN in StoichManager.h, because it may get moved around again at a later date, or used in a different file, etc. and would lead to hard-to-debug errors.

ischoegl · 2021-08-02T15:42:41Z

Nonetheless, I still think it's a good idea to make sure that config.h is included before checking CT_USE_SYSTEM_EIGEN in StoichManager.h, because it may get moved around again at a later date, or used in a different file, etc. and would lead to hard-to-debug errors.

Fair enough. I changed eigen_dense.h to include sparse (and renamed to eigen_defs.h), and included that in SolutionManager.h. I believe that way everything is taken care of.

While the method is part of the initialization, it is the final step required after all reactions are added to the Kinetics object.

While changes worked and can potentially create easier-to-follow code there is a performance penalty, as additional copy operations are needed.

Create flag to specify whether third body derivatives are included

Uses wrapper.h to simplify passing of data

* Move functions to header file * Make function names more consistent * Pass raw pointers to add flexibility

Enable 'exact' temperature derivatives for forward and net rates of progress, with the caveat that the derivative of the equilibrium constant is evaluated numerically.

Make naming more consistent and improve code performance and readability.

All derivatives are taken with respect to temperature and species mole fractions. In order to clarify (and shorten method names), Jacobian routines are renamed as follows: * Kinetics::*RopTemperatureDerivatives -> Kinetics::*RatesOfProgress_ddT * Kinetics::*RopSpeciesDerivatives -> Kinetics::*RatesOfProgress_ddC * Kinetics::*RateTemperatureDerivatives -> Kinetics::*Rates_ddT * Kinetics::*RateSpeciesDerivatives -> Kinetics::*Rates_ddC Essential jacobians methods are consolidated within GasKinetics

ischoegl · 2021-08-25T17:14:34Z

As this PR experienced substantial 'feature drift', I am closing this, and continuing with #1089 (which also avoids a meandering commit history).

ischoegl marked this pull request as draft August 1, 2021 00:38

ischoegl force-pushed the sparse-stoich-coeffs branch from ae147d6 to 318e5b3 Compare August 2, 2021 13:02

ischoegl force-pushed the sparse-stoich-coeffs branch 3 times, most recently from 9dc30cb to 6f79569 Compare August 2, 2021 15:41

ischoegl force-pushed the sparse-stoich-coeffs branch 12 times, most recently from da06abc to 8b5a95d Compare August 4, 2021 20:43

ischoegl changed the title ~~Sparse stoich coeffs~~ Refactor StoichManager for sparse Jacobian Calculations Aug 4, 2021

This comment has been minimized.

Sign in to view

ischoegl force-pushed the sparse-stoich-coeffs branch 5 times, most recently from 7d0aac7 to 9e0a6b9 Compare August 5, 2021 16:08

ischoegl mentioned this pull request Aug 5, 2021

Analytical Jacobian for Cantera Cantera/enhancements#19

Closed

ischoegl added 24 commits August 18, 2021 20:47

[Kinetics] Clarify nomenclature for Kinetics::finalizeSetup

82de5ed

While the method is part of the initialization, it is the final step required after all reactions are added to the Kinetics object.

[Kinetics] Revert to faster production rate calculation

7b4201d

While changes worked and can potentially create easier-to-follow code there is a performance penalty, as additional copy operations are needed.

[Kinetics] Combine APIs for species derivatives

fd232e7

[Kinetics] Use MappedSparseMatrix for Jacobian output

66f7d7b

[Kinetics] Use pointers to ThirdBodyCalc objects

27c88cb

[Kinetics] Set up sparse matrices in ThirdBodyCalc

7ab43e3

[Kinetics] Split GasKinetics::updateROP into parts

e16054d

[Kinetics] Expose existing temperature derivatives

84e9882

[Kinetics] Account for third-body concentrations in Jacobian

7d04da8

Create flag to specify whether third body derivatives are included

[UnitTest] Add tests for species Jacobians

a89bb29

[Kinetics] Implement temperature derivatives of rate expressions

742c237

[Unittest] Update and include temperature derivative checks

30d3af3

[Kinetics] Implement setters/getters for Jacobian settings

d57b28d

[Kinetics] Streamline Python API for temperature derivatives

f1a881b

Uses wrapper.h to simplify passing of data

[Kinetics] Streamline API for stoichiometric coefficient matrices

d3bae7a

[Kinetics] Streamline Python API for species derivatives

c54e340

[Kinetics] Shorten names of derivative getters

6decb70

[Kinetics] Optimize GasKinetics helper functions

82d9ef1

* Move functions to header file * Make function names more consistent * Pass raw pointers to add flexibility

[Python] Enable optional scipy.sparse output

aa4a35e

[Kinetics] Wrap up temperature derivatives

df0d6bb

Enable 'exact' temperature derivatives for forward and net rates of progress, with the caveat that the derivative of the equilibrium constant is evaluated numerically.

[Kinetics] Update nomenclature/code for GasKinetics service routines

f9dbd77

Make naming more consistent and improve code performance and readability.

Allow for custom NotImplementedError messages

4a49972

[Unittest] Expand and update tests to use streamlined Python API

53b764e

ischoegl force-pushed the sparse-stoich-coeffs branch from e1c9286 to 53b764e Compare August 19, 2021 02:51

This was referenced Aug 22, 2021

Small fixes and improvements / sparse coefficient matrices #1088

Merged

Sparse Jacobians for GasKinetics #1089

Merged

ischoegl closed this Aug 25, 2021

ischoegl deleted the sparse-stoich-coeffs branch August 25, 2021 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor StoichManager for sparse Jacobian Calculations#1081

Refactor StoichManager for sparse Jacobian Calculations#1081
ischoegl wants to merge 37 commits intoCantera:mainfrom
ischoegl:sparse-stoich-coeffs

ischoegl commented Aug 1, 2021 •

edited

Loading

Uh oh!

codecov bot commented Aug 1, 2021 •

edited

Loading

Uh oh!

bryanwweber commented Aug 2, 2021

Uh oh!

ischoegl commented Aug 2, 2021

Uh oh!

bryanwweber commented Aug 2, 2021 •

edited

Loading

Uh oh!

ischoegl commented Aug 2, 2021

Uh oh!

This comment has been minimized.

ischoegl commented Aug 25, 2021 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ischoegl commented Aug 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bryanwweber commented Aug 2, 2021

Uh oh!

ischoegl commented Aug 2, 2021

Uh oh!

bryanwweber commented Aug 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ischoegl commented Aug 2, 2021

Uh oh!

This comment has been minimized.

ischoegl commented Aug 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ischoegl commented Aug 1, 2021 •

edited

Loading

codecov bot commented Aug 1, 2021 •

edited

Loading

bryanwweber commented Aug 2, 2021 •

edited

Loading

ischoegl commented Aug 25, 2021 •

edited

Loading