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

save/restore ThermoPhase state assumes density is an independent variable #595

Closed
speth opened this issue Feb 7, 2019 · 1 comment · Fixed by #720
Closed

save/restore ThermoPhase state assumes density is an independent variable #595

speth opened this issue Feb 7, 2019 · 1 comment · Fixed by #720

Comments

@speth
Copy link
Member

speth commented Feb 7, 2019

The functions Phase::saveState and Phase::restoreState assume that temperature and density are the independent thermodynamic variables, which is not true for several of Cantera's thermo models. For these models, the save/restore feature does not work correctly. For example (in Python):

import cantera as ct
phase = ct.ThermoPhase('test_problems/cathermo/issp/IdealSolidSolnPhaseExample.xml')
T0, p0 = phase.TP # T0 = 500, p0 == 101325
z = phase.state # calls Phase::saveState
phase.TP = 600, 2*ct.one_atm
phase.state = z # calls Phase::restoreState
assert phase.T == T0, 'temperature'
assert phase.P == p0, 'pressure'

Here, the pressure is left at 2 atm, despite the call to restoreState. In other cases, the restoreState call will fail because the derived class overrides the setDensity function to throw an exception.

Among other things, this error contributes to making these phases difficult to use with the Solution class in Python and Matlab, where the default behavior of instantiating a dummy Transport object triggers calls to saveState and restoreState, with the latter failing.

This problem can be resolved by making the saveState and restoreState methods virtual and having them work with the appropriate set of independent variables for each phase type.

@ThanasisMattas
Copy link
Contributor

ThanasisMattas commented May 4, 2019

Hello! I would like to have a look on this issue. I think this could be handled parallel to this @todo entry commented in Phase.h:

@todo
- Make the concept of saving state vectors more general, so that it can
handle other cases where there are additional internal state variables,
such as the voltage, a potential energy, or a strain field.

So, possible overrides at derived classes will use their access methods for every other state variable, like voltage(). So, is just a virtual keyword enough here? And both methods can be overridden at the derived class. More specificly, I would like to ask if this requires any re-implementation of the overridden methods so that they handle the proper state variables (and, corresponding to the todo entry, to add any state variable to the state vector).

ischoegl pushed a commit to ischoegl/cantera that referenced this issue Oct 4, 2019
ischoegl pushed a commit to ischoegl/cantera that referenced this issue Oct 4, 2019
ischoegl pushed a commit to ischoegl/cantera that referenced this issue Oct 23, 2019
ischoegl pushed a commit to ischoegl/cantera that referenced this issue Nov 2, 2019
ischoegl pushed a commit to ischoegl/cantera that referenced this issue Nov 2, 2019
ischoegl pushed a commit to ischoegl/cantera that referenced this issue Nov 11, 2019
ischoegl pushed a commit to ischoegl/cantera that referenced this issue Nov 11, 2019
@speth speth closed this as completed in #720 Dec 6, 2019
speth pushed a commit that referenced this issue Dec 6, 2019
decaluwe pushed a commit to gkogekar/cantera that referenced this issue Dec 20, 2019
srikanthallu pushed a commit to srikanthallu/cantera that referenced this issue Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants