Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bryanwweber committed Jun 13, 2022
1 parent ee10985 commit 24021f8
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 97 deletions.
32 changes: 3 additions & 29 deletions include/cantera/thermo/Elements.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,41 +92,15 @@ const std::vector<std::string>& elementSymbols();
//! @since New in version 3.0
const std::vector<std::string>& elementNames();

//! Get a map with the element symbols as keys and weights as values.
//! Get a map with the element and isotope symbols and names as keys and weights as
//! values.
/*!
* This is a constant in the application so it is only generated once
* when it is first needed.
*
* @since New in version 3.0
*/
const std::map<std::string, double>& elementSymbolToWeight();

//! Get a map with the element names as keys and weights as values.
/*!
* This is a constant in the application so it is only generated once
* when it is first needed.
*
* @since New in version 3.0
*/
const std::map<std::string, double>& elementNameToWeight();

//! Get a map with the isotope symbols as keys and weights as values.
/*!
* This is a constant in the application so it is only generated once
* when it is first needed.
*
* @since New in version 3.0
*/
const std::map<std::string, double>& isotopeSymbolToWeight();

//! Get a map with the isotope names as keys and weights as values.
/*!
* This is a constant in the application so it is only generated once
* when it is first needed.
*
* @since New in version 3.0
*/
const std::map<std::string, double>& isotopeNameToWeight();
const std::map<std::string, double>& elementWeights();

//! Get the atomic weight of an element.
/*!
Expand Down
1 change: 1 addition & 0 deletions include/cantera/thermo/Species.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ unique_ptr<Species> newSpecies(const AnyMap& node);

//! Generate Species objects for each item (an AnyMap) in `items`.
std::vector<shared_ptr<Species>> getSpecies(const AnyValue& items);

}

#endif
20 changes: 11 additions & 9 deletions interfaces/cython/cantera/test/test_thermo.py
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,7 @@ def test_molecular_weight_with_electron(self):
def test_custom_element_weight_is_set_on_species(self):
gas = ct.Solution("ideal-gas.yaml", "element-override")
# Check that the molecular weight stored in the phase definition is the same
# as the one on the element
# as the one on the Species instance
self.assertNear(
gas["AR"].molecular_weights[0], gas.species("AR").molecular_weight
)
Expand All @@ -1265,15 +1265,15 @@ def test_species_can_be_added_to_phase(self):
})
# Access the molecular weight to make sure it's been computed by the Species
self.assertNear(s.molecular_weight, 39.95 * 2)
# This should not cause an error because the existing Ar definition is the
# default molecular weight
# This should not cause a warning because the Ar element definition in
# self.gas is the same as the default
self.gas.add_species(s)
self.assertNear(
self.gas["AR2"].molecular_weights[0],
self.gas.species("AR2").molecular_weight
)

def test_species_cannot_be_added_to_phase_custom_element(self):
def test_species_warns_when_changing_molecular_weight(self):
s = ct.Species.from_dict({
"name": "AR2",
"composition": {"Ar": 2},
Expand All @@ -1282,10 +1282,11 @@ def test_species_cannot_be_added_to_phase_custom_element(self):
# Access the molecular weight to make sure it's been computed by the Species
self.assertNear(s.molecular_weight, 39.95 * 2)
gas = ct.Solution("ideal-gas.yaml", "element-override")
# The error here is because the weight of the Argon element has been changed in
# The warning here is because the weight of the Argon element has been changed in
# the phase definition, but the molecular weight of the species has already been
# computed
with pytest.raises(ct.CanteraError, match="Molecular weight.*changed"):
# computed, so loading the phase definition changes the molecular weight on the
# species.
with pytest.warns(UserWarning, match="Molecular weight.*changing"):
gas.add_species(s)

def test_species_can_be_added_to_phase_custom_element(self):
Expand All @@ -1294,8 +1295,8 @@ def test_species_can_be_added_to_phase_custom_element(self):
"composition": {"Ar": 2},
"thermo": {"model": "constant-cp", "h0": 100}
})
# DO NOT access the molecular weight to make sure it's not been computed by the
# Species
# DO NOT access the molecular weight on the Species instance before adding it
# to the phase to make sure the weight has not been computed by the Species
gas = ct.Solution("ideal-gas.yaml", "element-override")
gas.add_species(s)
self.assertNear(
Expand Down Expand Up @@ -1736,6 +1737,7 @@ def setUpClass(cls):
cls.ar_num = ct.Element(18)

def test_element_multiple_possibilities(self):
# Carbon starts with Ca, the symbol for calcium.
carbon = ct.Element('Carbon')
self.assertEqual(carbon.name, 'carbon')
self.assertEqual(carbon.symbol, 'C')
Expand Down
66 changes: 16 additions & 50 deletions src/thermo/Elements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,59 +234,37 @@ const vector<string>& elementNames() {
return values;
}

template<typename T>
map<string, double> mapAtomicWeights(vector<T> data, bool symbol, bool name) {
map<string, double> mapAtomicWeights() {
map<string, double> symMap;

for (auto const& atom : data) {
if (symbol) {
symMap.emplace(atom.symbol, atom.atomicWeight);
} else if (name) {
symMap.emplace(atom.fullName, atom.atomicWeight);
}
for (auto const& atom : atomicWeightTable) {
symMap.emplace(atom.symbol, atom.atomicWeight);
symMap.emplace(atom.fullName, atom.atomicWeight);
}
for (auto const& isotope: isotopeWeightTable) {
symMap.emplace(isotope.symbol, isotope.atomicWeight);
symMap.emplace(isotope.fullName, isotope.atomicWeight);
}
return symMap;
}

const map<string, double>& elementSymbolToWeight() {
const static map<string, double> symMap = mapAtomicWeights(atomicWeightTable,
true, false);
return symMap;
}

const map<string, double>& elementNameToWeight() {
const static map<string, double> symMap = mapAtomicWeights(atomicWeightTable,
false, true);
return symMap;
}

const map<string, double>& isotopeSymbolToWeight() {
const static map<string, double> symMap = mapAtomicWeights(isotopeWeightTable,
true, false);
return symMap;
}

const map<string, double>& isotopeNameToWeight() {
const static map<string, double> symMap = mapAtomicWeights(isotopeWeightTable,
false, true);
const map<string, double>& elementWeights() {
const static map<string, double> symMap = mapAtomicWeights();
return symMap;
}

double getElementWeight(const std::string& ename)
{
const auto& elementSymbolMap = elementSymbolToWeight();
const auto& elementNameMap = elementNameToWeight();
const auto& isotopeSymbolMap = isotopeSymbolToWeight();
const auto& isotopeNameMap = isotopeNameToWeight();
const auto& elementMap = elementWeights();
double elementWeight = 0.0;
string symbol = trimCopy(ename);
string name = toLowerCopy(symbol);
auto search = elementSymbolMap.find(symbol);
if (search != elementSymbolMap.end()) {
auto search = elementMap.find(symbol);
if (search != elementMap.end()) {
elementWeight = search->second;
} else {
search = elementNameMap.find(name);
if (search != elementNameMap.end()) {
string name = toLowerCopy(symbol);
search = elementMap.find(name);
if (search != elementMap.end()) {
elementWeight = search->second;
}
}
Expand All @@ -296,18 +274,6 @@ double getElementWeight(const std::string& ename)
throw CanteraError("getElementWeight",
"element '{}' has no stable isotopes", ename);
}
search = isotopeSymbolMap.find(symbol);
if (search != isotopeSymbolMap.end()) {
elementWeight = search->second;
} else {
search = isotopeNameMap.find(name);
if (search != isotopeNameMap.end()) {
elementWeight = search->second;
}
}
if (elementWeight > 0.0) {
return elementWeight;
}
throw CanteraError("getElementWeight", "element not found: " + ename);
}

Expand Down
15 changes: 6 additions & 9 deletions src/thermo/Species.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ Species::~Species()
double Species::molecularWeight() {
if (m_molecularWeight == Undef) {
double weight = 0.0;
const auto& elements = elementSymbolToWeight();
const auto& isotopes = isotopeSymbolToWeight();
const auto& elements = elementWeights();
for (const auto& comp : composition) {
auto search = elements.find(comp.first);
if (search != elements.end()) {
Expand All @@ -50,11 +49,6 @@ double Species::molecularWeight() {
"element '{}' has no stable isotopes", comp.first);
}
weight += search->second * comp.second;
} else {
search = isotopes.find(comp.first);
if (search != isotopes.end() && search->second > 0) {
weight += search->second * comp.second;
}
}
}
setMolecularWeight(weight);
Expand All @@ -67,9 +61,12 @@ void Species::setMolecularWeight(double weight) {
double maxWeight = max(weight, m_molecularWeight);
double weight_cmp = fabs(weight - m_molecularWeight) / maxWeight;
if (weight_cmp > 1.0e-9) {
throw CanteraError(
warn_user(
"Species::setMolecularWeight",
"Molecular weight of a species cannot be changed."
"Molecular weight of species '{}' is changing from {} to {}.",
this->name,
m_molecularWeight,
weight
);
}
}
Expand Down

0 comments on commit 24021f8

Please sign in to comment.