-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Simplify creation of interface phases #1169
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1169 +/- ##
==========================================
+ Coverage 65.35% 65.37% +0.02%
==========================================
Files 315 317 +2
Lines 45999 46112 +113
Branches 19531 19603 +72
==========================================
+ Hits 30062 30148 +86
- Misses 13445 13461 +16
- Partials 2492 2503 +11
Continue to review full report at Codecov.
|
b02fcd1
to
c0f776f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR - this is a great addition, which should greatly simplify the instantiation of Interface
s + associated phases. Most of my comments are minor.
As a minor comment, the dictionary values
aren't easy to parse in examples, e.g.
tpb_a = ct.Interface("sofc.yaml", "tpb")
anode_surf, oxide_surf_a, anode_bulk = tpb_a.adjacent.values()
oxide_a, gas_a = oxide_surf_a.adjacent.values()
While it can be assumed that users are familiar with what is listed in the input files, this assumes knowledge of a specific order of adjacent phases. I overall believe that a dictionary is the right choice here, but would suggest to access entries by key (this should facilitate understanding in the case of a user just downloading an example file without ever looking into the YAML source).
This class provides access to the thermo and kinetics objects as the correct derived types, i.e. SurfPhase and InterfaceKinetics, so methods defined only for these derived types can be called easily.
The error message from the second reaction containing an error will not be encountered because parsing stops after the first invalid reaction when in debug mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @speth, this looks good to me. As said, I believe this will greatly simplify the creation of Interface
s (at the least, it makes this much easier to understand for new users).
The only remaining thing will be to update the corresponding YAML tutorial on Cantera/cantera-website.
Changes proposed in this pull request
adjacent-phases
YAML fieldSolution.adjacent(name)
oradjacent(index)
methods of the C++Solution
classSolution.adjacent
dict
in Python (keyed by phase name)adjacent-phases
field when serializing interfaces to YAMLadjacent-phases
field when converting mechansims from CK, CTI, or XML formatsPhase::setRoot
methodEdge(filename, phasename)
If applicable, fill in the issue number this pull request is fixing
Closes Cantera/enhancements#103
If applicable, provide an example illustrating new features this pull request is introducing
Python:
C++:
Checklist
scons build
&scons test
) and unit tests address code coverage