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.

Sign up for GitHub

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

Jump to bottom

Enable modification of user input data fields in Python #1031

Merged
merged 1 commit into from
Jun 7, 2021
Merged

Enable modification of user input data fields in Python #1031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions interfaces/cython/cantera/_cantera.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ cdef extern from "cantera/base/AnyMap.h" namespace "Cantera":
CxxAnyValue& operator[](string) except +translate_exception
cbool empty()
cbool hasKey(string)
void clear()
void update(CxxAnyMap& other, cbool)
string keys_str()
void applyUnits()

Expand Down Expand Up @@ -135,6 +137,7 @@ cdef extern from "cantera/thermo/SpeciesThermoInterpType.h":
void reportParameters(size_t&, int&, double&, double&, double&, double* const) except +translate_exception
int nCoeffs() except +translate_exception
CxxAnyMap parameters(cbool) except +translate_exception
CxxAnyMap& input()

cdef extern from "cantera/thermo/SpeciesThermoFactory.h":
cdef CxxSpeciesThermo* CxxNewSpeciesThermo "Cantera::newSpeciesThermoInterpType"\
Expand All @@ -154,6 +157,7 @@ cdef extern from "cantera/thermo/Species.h" namespace "Cantera":
double charge
double size
CxxAnyMap parameters(CxxThermoPhase*) except +translate_exception
CxxAnyMap input

cdef shared_ptr[CxxSpecies] CxxNewSpecies "newSpecies" (XML_Node&)
cdef vector[shared_ptr[CxxSpecies]] CxxGetSpecies "getSpecies" (XML_Node&)
Expand Down Expand Up @@ -187,6 +191,7 @@ cdef extern from "cantera/thermo/ThermoPhase.h" namespace "Cantera":
string type()
string phaseOfMatter() except +translate_exception
void getSpeciesParameters(string, CxxAnyMap&) except +translate_exception
CxxAnyMap& input()
speth marked this conversation as resolved.
Show resolved Hide resolved
string report(cbool, double) except +translate_exception
cbool hasPhaseTransition()
cbool isPure()
Expand Down Expand Up @@ -376,6 +381,7 @@ cdef extern from "cantera/kinetics/Reaction.h" namespace "Cantera":
string type()
void validate() except +translate_exception
CxxAnyMap parameters(cbool) except +translate_exception
CxxAnyMap input
int reaction_type
Composition reactants
Composition products
Expand Down Expand Up @@ -563,6 +569,7 @@ cdef extern from "cantera/transport/TransportData.h" namespace "Cantera":
cdef cppclass CxxTransportData "Cantera::TransportData":
CxxTransportData()
CxxAnyMap parameters(cbool) except +translate_exception
CxxAnyMap input

cdef cppclass CxxGasTransportData "Cantera::GasTransportData" (CxxTransportData):
CxxGasTransportData()
Expand Down
19 changes: 18 additions & 1 deletion interfaces/cython/cantera/base.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,22 @@ cdef class _SolutionBase:
def __get__(self):
return anymap_to_dict(self.base.parameters(True))

def update_user_data(self, data):
"""
Add the contents of the provided `dict` as additional fields when generating
YAML phase definition files with `write_yaml` or in the data returned by
`input_data`. Existing keys with matching names are overwritten.
speth marked this conversation as resolved.
Show resolved Hide resolved
"""
self.thermo.input().update(dict_to_anymap(data), False)

def clear_user_data(self):
"""
Clear all saved input data, so that the data given by `input_data` or
`write_yaml` will only include values generated by Cantera based on the
current object state.
"""
self.thermo.input().clear()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this distinguishes between data that the user has added using update_user_data and the data coming from the YAML file that was used to create the Solution. Can you clarify that for me? Thanks 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't distinguish - both are erased here. But once the object's been constructed, Cantera doesn't need a pristine copy of the data from the input file, so clearing this doesn't have any noticeable effect besides removing the auxiliary data, since the necessary fields will be calculated through the calls to getParameters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in principle, this does not support true round-tripping of an input file, right? If all the data are cleared, and any data computed by Cantera is not quite identical to the original file, the resulting generated file wouldn't be identical?

This may be a pedantic thing, but since the docstring says one thing, and the code actually does something else, I foresee potential problems as a result.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I think we're getting pretty far off the purpose of this PR, as this basically all about features that were introduced in #984 if not earlier, but I can try to provide a bit of an explanation.

We don't keep track of how an object has changed since it was instantiated from the input file, so any time you ask for the input_data or call write_yaml, that's being calculated on the fly. I don't think there's anything that promises to precisely round-trip an input file, and there are currently a variety of ways in which that isn't currently possible. Some of these will probably be addressed relatively soon, like the serialization of the file-level metadata (as requested in #1013), while others may not (like retaining comments in YAML files). The only promise that I think we should be trying to make regarding serialization is that you can generate an input file that will recreate an object in its current state, and I think that is currently the case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in principle, this does not support true round-tripping of an input file, right? If all the data are cleared, and any data computed by Cantera is not quite identical to the original file, the resulting generated file wouldn't be identical?

This may be a pedantic thing, but since the docstring says one thing, and the code actually does something else, I foresee potential problems as a result.

This is quite similar to what I had mentioned in one of my comments above (what does input data mean?). Either path forward is fine; I believe round-trip conversion would be simple to implement as stated earlier:

The implementation would be quite simple, where all that is needed is a second AnyMap member variable that holds information added after the objects are instantiated, plus some tweaks for logic in some of the getters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine if we consider changing/discussing the round-trip behavior out of scope for this PR. That said, I'd prefer if this docstring were a little more accurate to reflect what's actually happening. Since this very explicitly says that user-provided data will be cleared, it implies that's the only thing that's cleared. Maybe something like:

Clear auxiliary input data, including user-provided values. When write_yaml or input_data are called, Cantera will regenerate the input data set to reproduce the current object state.

I'm not sure if this is accurate though; I'm thinking of the case where a user may have manually, e.g., changed a species thermo entry (for example, to debug a discontinuous polynomial) and that data will be written out instead of the data that was originally used to create the Solution(). Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had been using "auxiliary" to mean data that is not used by Cantera, so there is no auxiliary input data that isn't user-provided. I guess this would be more precise:

Clear all saved input data, so that the data given by input_data or write_yaml will only include values generated by Cantera based on the current object state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds great to me! Thanks!


def write_yaml(self, filename, phases=None, units=None, precision=None,
skip_user_defined=None):
"""
Expand All @@ -251,7 +267,8 @@ cdef class _SolutionBase:
the right of the decimal point. The default is 15 digits.
:param skip_user_defined:
If `True`, user-defined fields which are not used by Cantera will
be stripped from the output.
be stripped from the output. These additional contents can also be
controlled using the `update_user_data` and `clear_user_data` functions.
"""
Y = YamlWriter()
Y.add_solution(self)
Expand Down
16 changes: 16 additions & 0 deletions interfaces/cython/cantera/reaction.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,22 @@ cdef class Reaction:
def __get__(self):
return anymap_to_dict(self.reaction.parameters(True))

def update_user_data(self, data):
"""
Add the contents of the provided `dict` as additional fields when generating
YAML phase definition files with `Solution.write_yaml` or in the data returned
by `input_data`. Existing keys with matching names are overwritten.
"""
self.reaction.input.update(dict_to_anymap(data), False)

def clear_user_data(self):
"""
Clear all saved input data, so that the data given by `input_data` or
`Solution.write_yaml` will only include values generated by Cantera based on
the current object state.
"""
self.reaction.input.clear()

def __repr__(self):
return '<{}: {}>'.format(self.__class__.__name__, self.equation)

Expand Down
16 changes: 16 additions & 0 deletions interfaces/cython/cantera/speciesthermo.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,22 @@ cdef class SpeciesThermo:
def __get__(self):
return anymap_to_dict(self.spthermo.parameters(True))

def update_user_data(self, data):
"""
Add the contents of the provided `dict` as additional fields when generating
YAML phase definition files with `Solution.write_yaml` or in the data returned
by `input_data`. Existing keys with matching names are overwritten.
"""
self.spthermo.input().update(dict_to_anymap(data), False)

def clear_user_data(self):
"""
Clear all saved input data, so that the data given by `input_data` or
`Solution.write_yaml` will only include values generated by Cantera based on
the current object state.
"""
self.spthermo.input().clear()

def cp(self, T):
"""
Molar heat capacity at constant pressure [J/kmol/K] at temperature *T*.
Expand Down
34 changes: 34 additions & 0 deletions interfaces/cython/cantera/test/test_composite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,19 @@ def test_input_data_simple(self):
self.assertEqual(data['kinetics'], 'gas')
self.assertEqual(data['transport'], 'mixture-averaged')

def test_input_data_user_modifications(self):
gas = ct.Solution("h2o2.yaml")
data1 = gas.input_data
gas.update_user_data({"foo": True}) # should get overwritten
extra = {"foo": [1.2, 3.4], "bar": [[1, 2], [3, 4]]}
gas.update_user_data(extra)
data2 = gas.input_data
self.assertEqual(extra["foo"], data2["foo"])
self.assertEqual(extra["bar"], data2["bar"])
gas.clear_user_data()
data3 = gas.input_data
self.assertEqual(data1, data3)

def test_input_data_state(self):
gas = ct.Solution('h2o2.yaml', transport_model=None)
data = gas.input_data
Expand Down Expand Up @@ -569,6 +582,27 @@ def test_yaml_inconsistent_species(self):
with self.assertRaisesRegex(ct.CanteraError, "different definitions"):
gas.write_yaml('h2o2-error.yaml', phases=gas2)

def test_yaml_user_data(self):
gas = ct.Solution("h2o2.yaml")
extra = {"spam": {"A": 1, "B": 2}, "eggs": [1, 2.3, 4.5]}
gas.update_user_data(extra)
S = gas.species(2)
S.update_user_data({"foo": "bar"})
S.transport.update_user_data({"baz": 1234.5})
S.thermo.update_user_data({"something": (False, True)})
gas.reaction(5).update_user_data({"baked-beans": True})

gas.write_yaml("h2o2-generated-user-data.yaml")
gas2 = ct.Solution("h2o2-generated-user-data.yaml")
speth marked this conversation as resolved.
Show resolved Hide resolved
data2 = gas2.species(2).input_data

self.assertEqual(gas2.input_data["spam"], extra["spam"])
self.assertEqual(gas2.input_data["eggs"], extra["eggs"])
self.assertEqual(data2["foo"], "bar")
self.assertEqual(data2["transport"]["baz"], 1234.5)
self.assertEqual(data2["thermo"]["something"], [False, True])
self.assertTrue(gas2.reaction(5).input_data["baked-beans"])


class TestSpeciesSerialization(utilities.CanteraTest):
def test_species_simple(self):
Expand Down
16 changes: 16 additions & 0 deletions interfaces/cython/cantera/thermo.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,22 @@ cdef class Species:
cdef CxxThermoPhase* phase = self._phase.thermo if self._phase else NULL
return anymap_to_dict(self.species.parameters(phase))

def update_user_data(self, data):
"""
Add the contents of the provided `dict` as additional fields when generating
YAML phase definition files with `Solution.write_yaml` or in the data returned
by `input_data`. Existing keys with matching names are overwritten.
"""
self.species.input.update(dict_to_anymap(data), False)

def clear_user_data(self):
"""
Clear all saved input data, so that the data given by `input_data` or
`Solution.write_yaml` will only include values generated by Cantera based on
the current object state.
"""
self.species.input.clear()

def __repr__(self):
return '<Species {}>'.format(self.name)

Expand Down
16 changes: 16 additions & 0 deletions interfaces/cython/cantera/transport.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,22 @@ cdef class GasTransportData:
def __get__(self):
return anymap_to_dict(self.data.parameters(True))

def update_user_data(self, data):
"""
Add the contents of the provided `dict` as additional fields when generating
YAML phase definition files with `Solution.write_yaml` or in the data returned
by `input_data`. Existing keys with matching names are overwritten.
"""
self.data.input.update(dict_to_anymap(data), False)

def clear_user_data(self):
"""
Clear all saved input data, so that the data given by `input_data` or
`Solution.write_yaml` will only include values generated by Cantera based on
the current object state.
"""
self.data.input.clear()

property geometry:
"""
Get/Set the string specifying the molecular geometry. One of `atom`,
Expand Down