-
Notifications
You must be signed in to change notification settings - Fork 5
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
Revise YAML species definitions to support multiple thermo models #20
Comments
I'm not sure that the differentiation between As an aside, it is not clear why the current implementation uses top-level entries PS: there may be an Option 4, which eliminates the |
I find that there's very little that's actually clear when it comes to the I had hoped that the name
The
The two different notations is kind of the point of this option. The
Changing the location of these fields is one of the two main points of this enhancement proposal.
This would make it difficult to create |
@speth ... your explanations confirm some of my suspicions (especially regarding
... would it make sense to merge - name: NaCl(aq) # species for Debye-Huckel phase
composition: {Na: 1, Cl: 1}
Debye-Huckel:
equation-of-state:
model: constant-volume
molar-volume: 1.3
electrolyte-species-type: weak-acid-associated
weak-acid-charge: -1.0 which would work as long as there aren't multiple |
An |
@speth ... what I suggested above was mostly in direct response to your explanation. I was also thinking about how things are instantiated from the UI (the user implicitly specifies - name: NaCl(aq) # species for Debye-Huckel phase
composition: {Na: 1, Cl: 1}
Debye-Huckel:
equation-of-state:
model: constant-volume
molar-volume: 1.3
electrolyte-species-type: weak-acid-associated
weak-acid-charge: -1.0
# other CPStandardStateTP derived phase models I acknowledge that there may be redundant |
So if you wanted to use this species definition with additional phase thermo models, you would have to add a new section to the species definition for each of them, even if all they required was the specification of a constant molar volume? That loses the level of reusability that even the current structure has. I think it's more likely to want to be able to re-use the same My question about the other phase types that use PDSS models was mainly concerned with the fact that with the exception of Debye-Huckel, the other models that use - name: KCl(l)
composition: {K: 1, Cl: 1}
Margules:
equation-of-state:
model: constant-volume
molar-volume: 37.57 cm^3/gmol This structure also makes it more difficult to construct the I should also clarify that I don't think that these top-level sections should necessarily be paired 1 to 1 with phase |
@speth ... as long as there's a single However,
is there a way to make the YAML input more self-explanatory? There appears to be a shifting meaning of what is an equation of state, and what is a sub-(or super?)-model of an equation of state. Imho a cleaner delineation of meanings would go a long way (see also issue #6). As an example, I'm wondering whether it wouldn't make sense to rename |
There have been some changes terminology, which I had hoped were an improvement over what was used in XML (and is still pervasive in the C++ code). While I understand the utility of referencing thermodynamic properties to a state that may not correspond to the canonical thermodynamic standard state of the pure substance at 298.15 K and 1 bar (i.e. referencing solutes to infinite dilution in a particular solvent), I still cannot parse what it means for the standard state to depend on pressure and temperature. However, all of the so-called PDSS models do provide a pressure-volume-temperature relationship for the species, while in many cases (though admittedly not all), the additional information needed to compute thermodynamic properties (i.e. enthalpy, entropy, etc.) is not part of the PDSS model. This was my motivation for the use of the term I guess one bit of complexity that has arisen here comes from the fact that for equations of state like Redlich-Kwong, there are additional species-specific properties needed, but that model doesn't happen to use the |
OK, I'm starting to see what the motivation is, and can agree on the definition of equation-of-state (although it is unfortunately not clear in the C++ implementation, and R-K etc. straddle definitions). I'm still not convinced that Options 1 and 2 do better, as Nomenclature aside it may make sense to go with Option 3 as it best reflects the interdependence of classes. I.e. YAML could be used as a starting point for a clarification of the interdependence of equations of state, phases, etc, in documentation (also addressing issue #6). Users typically don't know how things are implemented, so the main point is to be clear in how objects depend on each other in an intuitive interface. Whether or not C++ is clear is a separate issue. Alternatively, there are solutions that combine aspects of Options 1/2 and 3 (i.e. clarify that |
OK, I can see that putting the extra In that case, I can see the appeal of taking the approach of Options 1 or 2 for allowing multiple sets of |
👍 Yes, at least to me that would clarify the interdependence.
That’s not necessarily a disadvantage if it clarifies the overall structure / interdependence. There are likely other tweaks, but that’s as far as my feedback goes. |
OK, thanks for the discussion. I've updated some of the pros/cons, and introduced Option 4 (combination of 1 and 3) to provide an easier starting point for others who might want to offer their thoughts. |
Well, this quickly escalated past something I can readily comment on 🤣 (i.e. the details of models like Regardless, comment I will:
The last thing on my mind which is partially relevant: the Lastly, the acentric factor is sort of a headache, regardless, in that it can be both a transport property (for |
Lol - the conversation evolved beneath my feet, as I was typing that. Just wanted to note that Option 4 is equivalently suitable to Option 1, from my standpoint. |
Thanks, @decaluwe. I was vaguely aware that the acentric factor came into play in the Soave-Redlich-Kwong equation, but didn't realize that it was used in the Peng-Robinson equation as well. Given that it is a physical property of the species, not just a model parameter, and that it is used in several equations of state as well as transport models, I think there's a case to be made that it could be included as a top-level field in the species definition. Or perhaps, given the close relationship to the critical properties, as part of a field that also includes those values. |
A minor remark following up on @speth's last comment: using that rationale, |
I get the impression that |
Yeah, the acentric factor is used to calculate kappa, which is used to calculate alpha, which shows up in the Peng-Robinson equation. I think the answer of "what to do with the acentric factor?" depends on if there will be other similar such properties. Having a loose If not, then I would say that storing it with the critical properties is a good approach. This probably simplifies the implementation, too. If no P-R constants ( One downside, regardless of how we answer this question at the YAML level: I need to add acentric factors for all those species in the critProperties.xml database. Also, I'm guessing I ought to convert this database to YAML? |
A Yes, we should convert the |
👍 on the |
@speth Since Cantera/cantera#795 is merged, what's the status of this issue now? Can it be closed? |
Most of this is resolved by that PR, but I didn't want to lose this last idea of creating a |
I'm closing this since #107 now covers the one remaining issue of finding a home for critical property information. |
Abstract
The aim of this enhancement is to improve the organization of thermo data within YAML "species" entries to allow better re-use with multiple phase thermo models and simplify serialization. Comments on the possible solutions would be greatly appreciated.
Motivation
Two problematic cases have been identified with how the species entries are currently organized. One comes from a discussion on Cantera/cantera#641 about how to store model-specific parameters for multiple models (i.e. Redlich-Kwong and Peng-Robinson coefficients) with a single species definition. The other was one I noticed while working on the implementation of #11, which is that (a) species data needed by the
ThermoPhase
is almost always in theequation-of-state
field, with the exception of several fields used in the Debye-Huckel model and (b) we are using theequation-of-state
field both for setting upThermoPhase
objects and for setting upPDSS
objects.Two species definitions for illustration:
Possible Solutions
There are several possible solutions, with different pros and cons. In the examples below, the
thermo
field for each species has been elided for simplicity.Update: Based on the discussion with @ischoegl some of the pros/cons have been updated, and a 4th option has been introduced. I think the newly-introduced Option 4 is my current preference.
Option 1: Allow
equation-of-state
to be a list, and put all phase-specific thermo data an entry in this listPros:
equation-of-state
fieldCons:
ThermoPhase
andPDSS
models, and due to the fact that theequation-of-state
field can be either a map or a list of maps.equation-of-state
entries are used simultaneouslyOption 2: Make
equation-of-state
a map of mapsPros:
Cons:
equation-of-state
map.transport
andthermo
nodesequation-of-state
fieldequation-of-state
entries are used simultaneouslyOption 3: Use
equation-of-state
only forPDSS
setup, and use a per-model top level field for phase-specific dataPros:
ThermoPhase
andPDSS
parameters makes implementation straightforwardCons:
PDSS
model)Option 4: Allow lists in the
equation-of-state
section for multiple sets of equation of state parameters, of which one will be used with a particular phase. Create a new top-level key for Debye-Huckel parameters. This is a mix of options 1 and 3.Pros:
Cons:
equation-of-state
to be either a map or a list of maps.The text was updated successfully, but these errors were encountered: