Address review comments

include/cantera/thermo/Elements.h

@@ -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.
 /*!

include/cantera/thermo/Species.h

@@ -68,11 +68,7 @@ class Species
      * method the first time the molecularWeight() method is called if the species has
      * not been added to a phase.
      *
-     * @param weight: The weight of this species to assign, unless `compute` is `true`
-     * @param compute: If `true`, the weight will be computed from standard element
-     * weights from Elements.cpp, and the value of `weight` is ignored.
-     * @exception CanteraError If the molecular weight has already been set and the updated value
-     * is not close to the existing value.
+     * @param weight: The weight of this species to assign
      *
      * @since New in version 3.0
      */
@@ -98,6 +94,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

interfaces/cython/cantera/test/test_thermo.py

@@ -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
         )
@@ -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},
@@ -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):
@@ -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(
@@ -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')

src/thermo/Elements.cpp

@@ -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;
         }
     }
@@ -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);
 }
 

src/thermo/Phase.cpp

@@ -834,8 +834,6 @@ bool Phase::addSpecies(shared_ptr<Species> spec) {
     // weight to avoid dividing by zero.
     wt = std::max(wt, Tiny);
 
-    // Since this method can throw, all the modifications of the member variables
-    // should be done after this method call to avoid inconsistent state.
     spec->setMolecularWeight(wt);
 
     m_molwts.push_back(wt);

src/thermo/Species.cpp

@@ -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()) {
@@ -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);
@@ -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
             );
         }
     }