Cantera / cantera

Chemical kinetics, thermodynamics, and transport tool suite
https://cantera.org
Other
582 stars 342 forks source link

Assorted C++ / SCons cleanup #1574

Closed speth closed 10 months ago

speth commented 11 months ago

Changes proposed in this pull request

Checklist

codecov[bot] commented 11 months ago

Codecov Report

Merging #1574 (c1eec22) into main (25851e7) will decrease coverage by 0.02%. The diff coverage is 59.57%.

:exclamation: Current head c1eec22 differs from pull request most recent head 3b30d09. Consider uploading reports for the commit 3b30d09 to get more accurate results

@@            Coverage Diff             @@
##             main    #1574      +/-   ##
==========================================
- Coverage   70.53%   70.52%   -0.02%     
==========================================
  Files         379      379              
  Lines       59110    59122      +12     
  Branches    21232    21238       +6     
==========================================
+ Hits        41695    41697       +2     
- Misses      14340    14352      +12     
+ Partials     3075     3073       -2     
Files Changed Coverage Δ
include/cantera/base/AnyMap.h 80.00% <ø> (ø)
include/cantera/base/Array.h 96.00% <ø> (+13.24%) :arrow_up:
include/cantera/base/ValueCache.h 100.00% <ø> (ø)
include/cantera/kinetics/InterfaceKinetics.h 87.50% <ø> (ø)
include/cantera/kinetics/Kinetics.h 32.66% <ø> (ø)
include/cantera/kinetics/ReactionPath.h 67.44% <ø> (ø)
include/cantera/kinetics/ThirdBodyCalc.h 98.38% <ø> (ø)
include/cantera/numerics/BandMatrix.h 0.00% <ø> (ø)
include/cantera/numerics/GeneralMatrix.h 15.38% <ø> (ø)
...nclude/cantera/thermo/CoverageDependentSurfPhase.h 100.00% <ø> (+32.00%) :arrow_up:
... and 10 more

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

ischoegl commented 10 months ago

PS: Regarding the removal of ReactorSurface from Wall it may be worth revisiting in the future.

speth commented 10 months ago

PS: Regarding the removal of ReactorSurface from Wall it may be worth revisiting in the future.

I don't think so. The only thing that Wall and ReactorSurface have in common is that they both have an area. Wall objects represent interactions between two reactors, while ReactorSurface objects represent heterogeneous kinetics in a single reactor. Separating these capabilities led to a significant simplification in setting up networks with ReactorSurfaces, since you don't need to define a separate Reactor or Reservoir to insert the wall between just to be able to add a reacting surface.

ischoegl commented 10 months ago

I guess I misspoke: it's actually WallBase I had in mind, see doxygen wallGroup. While this may still not make sense, there's also this old issue that we may be able to resolve at some point ... https://github.com/Cantera/enhancements/discussions/92