search

Cantera / cantera

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

Fix usage of addOrderingRules in SolutionArray #1781

Closed speth closed 3 months ago

speth commented 3 months ago

Changes proposed in this pull request

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

Closes #1776

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

Checklist

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 77.77778% with 8 lines in your changes missing coverage. Please review.

Project coverage is 73.24%. Comparing base (dd26e18) to head (8bcc7a6). Report is 9 commits behind head on main.

Files Patch % Lines
src/base/AnyMap.cpp 70.58% 0 Missing and 5 partials :warning:
src/base/SolutionArray.cpp 76.92% 0 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1781 +/- ## ======================================= Coverage 73.23% 73.24% ======================================= Files 381 381 Lines 54376 54375 -1 Branches 9253 9251 -2 ======================================= + Hits 39824 39826 +2 + Misses 11579 11578 -1 + Partials 2973 2971 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

speth commented 3 months ago

Unfortunately, this PR reverts my earlier fix to the previous situation, where overwriting an existing SolutionArray scrambles the order of entries.

Not quite. The order of output is now different than what it was before, but it is now stable when overwriting the file. I've updated the corresponding test to check that this is indeed the case.

For a 1D flame, the output now comes out in the following order:

  flame:
    type: free-flow
    size: 9
    basis: mass
    components: [grid, velocity, T, D, H2, H, O, O2, OH, H2O, HO2, H2O2, AR, N2]
    points: 9
    energy-enabled: true
    phase:
      name: ohmech
      source: /Users/speth/src/cantera/build/python/cantera/data/h2o2.yaml
    radiation-enabled: false
    species-enabled: true
    transport-model: mixture-averaged
    Soret-enabled: false
    flux-gradient-basis: 1
    refine-criteria:
      ratio: 3.0
      slope: 0.06
      curve: 0.12
      prune: 0.0
      grid-min: 1.0e-10
      max-points: 1000
    tolerances:
      transient-abstol: 1.0e-11
      steady-abstol: 1.0e-09
      transient-reltol: 1.0e-04
      steady-reltol: 1.0e-04
    fixed-point:
      location: 0.0105
      temperature: 698.1678485027377
    grid: [0.0, 6.0e-03, 9.0e-03, 0.0105, 0.012, 0.015, 0.018, 0.024, 0.03]
    velocity: [0.3448108359476526, 0.3465608656848906, 0.3770754165767459,
    0.5740023649233583, 1.364440990996746, 1.439569578030446,
    1.450116128788632, 1.452935503745143, 1.452935503745178]
    D: [1.004145957804174, 0.9990718314487402, 0.9182237865090841,
    0.6032044854535895, 0.2537620765431486, 0.2405193521271233,
    0.2387706120988350, 0.2383081252938835, 0.2383081252938287]
    T: [400.0, 403.1281230365552, 445.1340622371536, 698.1678485027377,
    1717.667237539325, 1819.506593089243, 1833.946369770278,
    1837.803402275211, 1837.803402275211]
    H2: [9.466124468899828e-03, 9.282907545332970e-03, 8.219365106913516e-03,
    5.573672033086918e-03, 1.659105275821406e-04, 2.220294855777577e-05,
    6.143983384802635e-06, 2.911456291310724e-06, 2.911456291310724e-06]
    H: [1.179218242823033e-15, 7.789514857054084e-15, 2.921328219627074e-11,
    1.809466438706173e-07, 4.695488413457595e-05, 2.476002464123360e-06,
    3.627949117616036e-07, 1.210646613146704e-07, 1.210646613146704e-07]
    ...

This matches the order that things are added to the output AnyMap: metadata first, then "extra" fields which include the grid, velocity, and any solution components that aren't part of the thermodynamic state, followed by the thermodynamic state variables, then with a few specific fields moved to the beginning. The order that T, D, and Y come out is dictated by the order provided by phase->nativeState(), which uses std::map and therefore returns items in alphabetical order. If you want them in the order used by ThermoPhase, then I guess we could use nativeMode instead?

ischoegl commented 3 months ago

The order that T, D, and Y come out is dictated by the order provided by phase->nativeState(), which uses std::map and therefore returns items in alphabetical order. If you want them in the order used by ThermoPhase, then I guess we could use nativeMode instead?

Thanks for reminding me of this difference. Yes, things probably should be added in the same order as they show up in components, i.e. 

[grid, velocity, T, D, H2, H, O, O2, OH, H2O, HO2, H2O2, AR, N2]

which itself can be added at a later point so it immediately precedes the grid entry?

Beyond, we can probably banish phase to the tail? I honestly was too focused on making HDF5 behave when I implemented this and didn't pick up on those subtle differences.

speth commented 3 months ago

Thanks for reminding me of this difference. Yes, things probably should be added in the same order as they show up in components, i.e.

[grid, velocity, T, D, H2, H, O, O2, OH, H2O, HO2, H2O2, AR, N2]

I've pushed an update that implements this, with components showing up just before grid.

Beyond, we can probably banish phase to the tail? I honestly was too focused on making HDF5 behave when I implemented this and didn't pick up on those subtle differences.

I don't think we need to be too fussy about the formatting in this file. That said, I kind of like having the array data come last, as it can be quite large if you have a lot of grid points and species. The other settings for the 1D domain are much smaller by comparison, so it's kind of nice to see them up front.