-
-
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
Prelude to YAML serialization #795
Conversation
Codecov Report
@@ Coverage Diff @@
## master #795 +/- ##
=========================================
+ Coverage 71.44% 71.54% +0.1%
=========================================
Files 372 372
Lines 43952 44348 +396
=========================================
+ Hits 31400 31730 +330
- Misses 12552 12618 +66
Continue to review full report at Codecov.
|
For entries that are lists of maps in the general case but often only contain one item, it is convenient to allow them to be provided as just a map. This modifies methods used to work with vectors of AnyMap objects to handle the case where the child object is just a map.
This allows for error checking at the time of adding the alias, as well as providing a public interface for adding aliases.
The first phase in a Kinetics object is not necessarily the reacting phase for Kinetics objects loaded from CTI/XML files, but is for YAML files.
5579ea4
to
8772272
Compare
8772272
to
5545939
Compare
Didn't mean to request/submit review
Ensure that the name returned by Sub::name() is a name that can be used to reconstruct a substance of the same type. Make phase names in liquidvapor.yaml match the substance model names.
After calculating properties using finite differences (e.g. cp), the fluid was returned to its original state by setting the state in terms of pressure and temperature. However, because setting the state using these variables is an iterative calculation with some error tolerance, the state was not restored exactly. By restoring the state based on the initial temperature and density, the restored state is identical to the initial state.
5545939
to
cdef8c7
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.
Looks good to me, other than a few copy-editing things.
doc/sphinx/yaml/phases.rst
Outdated
One of ``solvent``, ``charged-species``, ``weak-acid-associated``, | ||
``strong-acid-associated``, ``polar-neutral``, or ``nonpolar-neutral``. | ||
The types ``solvent``, ``charged-species``, and ``nonpolar-neutral`` can be | ||
inferred automatically. |
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.
Can you add something here about how these types are automatically inferred?
doc/sphinx/yaml/phases.rst
Outdated
inferred automatically. | ||
|
||
``weak-acid-charge`` | ||
Charge to use for species can break apart into charged species. |
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.
Charge to use for species can break apart into charged species. | |
Charge to use for species that can break apart into charged species. |
doc/sphinx/yaml/species.rst
Outdated
the ``ideal-gas`` model is assumed. | ||
A mapping or list of mappings. Each mapping contains an equation of state | ||
model specification for the species, any parameters for that model, and any | ||
parameters for interactions with other species. :ref:`sec-yaml-species-eos`. |
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.
Is the reference intended to be a complete sentence here?
Checklist
scons build
&scons test
) and unit tests address code coverageTo make the upcoming PR which implements serialization to the YAML format a bit smaller and more manageable, I have extracted a number of commits which resolve some issues that I encountered and interface additions that I found were needed while implementing serialization, which can be considered separately.
Changes proposed in this pull request
AnyMap
class, including:clear()
method__file__
) when iteratingAnyMap
treeequation-of-state
field to allow multiple model options