Repository: MolSSI/QCElemental

Issue:
Floating point number allowed for molecular charge but not molecular multiplicity
**Describe the bug**
<!-- A clear and concise description of what the bug is. -->
QCElemental allows floating point numbers for molecular charges, however when computing the spin with the fractional electron there is no way to represent the molecular multiplicity with just an integer.

**To Reproduce**
<!-- Code to reproduce the behavior: Please include the results of:-->
```python
import qcelemental as qcel
mol = qcel.models.Molecule(
    geometry=[0.0, 0.0, 0.5, 0.0, 0.0, -0.5],
    symbols=["H", "He"],
    molecular_charge=0.5,
    molecular_multiplicity=1.5,  # not possible
)
```

**Expected behavior**
<!-- A clear and concise description of what you expected to happen. -->
Consistent behavior for specifying molecular charge and molecular multiplicity.

**Additional context**
<!-- Add any other context about the problem here. -->

Hints from issue comments:
@loriab: I'll make sure to bring this up to @bennybp to see if the database is ready for a type change.

Solution patch:
diff --git a/.github/workflows/CI.yaml b/.github/workflows/CI.yaml
index aa949f3..8aca4c9 100644
--- a/.github/workflows/CI.yaml
+++ b/.github/workflows/CI.yaml
@@ -17,7 +17,8 @@ jobs:
       matrix:
         python-version: ["3.7", "3.9", "3.11", "3.12"]
         pydantic-version: ["1", "2"]
-        runs-on: [ubuntu-latest, windows-latest]
+        # runs-on: [ubuntu-latest, windows-latest]
+        runs-on: [ubuntu-22.04, windows-latest]  # until drop py37
         exclude:
           - runs-on: windows-latest
             pydantic-version: "1"
diff --git a/.github/workflows/Lint.yml b/.github/workflows/Lint.yml
index c337352..c52ea9a 100644
--- a/.github/workflows/Lint.yml
+++ b/.github/workflows/Lint.yml
@@ -14,7 +14,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v4
         with:
-          python-version: "3.7"
+          python-version: "3.8"
       - name: Install black
         run: pip install "black>=22.1.0,<23.0a0"
       - name: Print code formatting with black (hints here if next step errors)
@@ -29,7 +29,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v4
         with:
-          python-version: "3.7"
+          python-version: "3.8"
       - name: Install poetry
         run: pip install poetry
       - name: Install repo
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 76edbc6..0cd1e4f 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -19,18 +19,19 @@ Changelog
 .. Misc.
 .. +++++
 
-- (:pr:`340`, :issue:`330`) Add molecular charge and multiplicity to Molecule repr formula,
-  so neutral singlet unchanged but radical cation has '2^formula+'.
-
 
 0.29.0 / 2024-MM-DD (Unreleased)
 --------------------------------
 
 Breaking Changes
 ++++++++++++++++
+- (:pr:`341`) `packaging` is now a required dependency.
 
 New Features
 ++++++++++++
+- (:pr:`350`, :pr:`318`, :issue:`317`) Make behavior consistent between molecular_charge/
+  fragment_charges and molecular_multiplicity/fragment_multiplicities by allowing floating point
+  numbers for multiplicities. @awvwgk
 - (:pr:`360`) ``Molecule`` learned new functions ``element_composition`` and ``molecular_weight``.
   The first gives a dictionary of element symbols and counts, while the second gives the weight in amu.
   Both can access the whole molecule or per-fragment like the existing ``nelectrons`` and
@@ -38,13 +39,30 @@ New Features
 
 Enhancements
 ++++++++++++
+- (:pr:`340`, :issue:`330`) Add molecular charge and multiplicity to Molecule repr formula,
+  so neutral singlet unchanged but radical cation has '2^formula+'. @awvwgk
+- (:pr:`341`) Use `packaging` instead of deprecated `setuptools` to provide version parsing for
+  `qcelemental.util.parse_version` and `qcelemental.util.safe_version`. This behaves slightly
+  different; "v7.0.0+N/A" was processed ok before but fails with newer version. @berquist
+- (:pr:`343`) Molecular and fragment multiplicities are now always enforced to be >=1.0. Previously
+  this wasn't checked for `Molecule(..., validate=False)`. Error messages will change sometimes
+  change for `validate=True` (run by default).
+- (:pr:`343`) `qcelemental.molparse` newly allows floats that are ints (e.g., 1.0) for multiplicity.
+  Previously it would raise an error about not being an int.
+- (:pr:`337`) Solidify the (unchanged) schema_name for `QCInputSpecification` and `AtomicResult`
+  into Literals where previously they had been regex strings coerced into a single name. The literals
+  allow pydantic to discriminate models, which benefits GeneralizedOptimizationInput/Result in
+  QCManyBody/QCEngine/OptKing. The only way this can interfere is if schema producers have whitespace
+  around `schema_name` for these models or if any `AtomicResult`s are still using "qc_schema_output",
+  which looks to have only been added for compatibility with pre-pydantic QCSchema.
 
 Bug Fixes
 +++++++++
 
 Misc.
 +++++
-- (:pr:`342`) Update some docs settings and requirements for newer tools.
+- (:pr:`344`, :issue:`282`) Add a citation file since QCElemental doesn't have a paper. @lilyminium
+- (:pr:`342`, :issue:`333`) Update some docs settings and requirements for newer tools.
 - (:pr:`353`) copied in pkg_resources.safe_version code as follow-up to Eric switch to packaging as both nwchem and gamess were now working. 
 the try_harder_safe_version might be even bettter
 
diff --git a/qcelemental/models/molecule.py b/qcelemental/models/molecule.py
index 63070d5..215db12 100644
--- a/qcelemental/models/molecule.py
+++ b/qcelemental/models/molecule.py
@@ -185,7 +185,7 @@ class Molecule(ProtoModel):
         description="Additional comments for this molecule. Intended for pure human/user consumption and clarity.",
     )
     molecular_charge: float = Field(0.0, description="The net electrostatic charge of the molecule.")  # type: ignore
-    molecular_multiplicity: int = Field(1, description="The total multiplicity of the molecule.")  # type: ignore
+    molecular_multiplicity: float = Field(1, description="The total multiplicity of the molecule.")  # type: ignore
 
     # Atom data
     masses_: Optional[Array[float]] = Field(  # type: ignore
@@ -257,7 +257,7 @@ class Molecule(ProtoModel):
         "if not provided (and :attr:`~qcelemental.models.Molecule.fragments` are specified).",
         shape=["nfr"],
     )
-    fragment_multiplicities_: Optional[List[int]] = Field(  # type: ignore
+    fragment_multiplicities_: Optional[List[float]] = Field(  # type: ignore
         None,
         description="The multiplicity of each fragment in the :attr:`~qcelemental.models.Molecule.fragments` list. The index of this "
         "list matches the 0-index indices of :attr:`~qcelemental.models.Molecule.fragments` list. Will be filled in based on a set of "
@@ -421,12 +421,16 @@ class Molecule(ProtoModel):
             n = len(values["fragments_"])
             if len(v) != n:
                 raise ValueError("Fragment Multiplicities must be same number of entries as Fragments")
+        v = [(int(m) if m.is_integer() else m) for m in v]
         if any([m < 1.0 for m in v]):
             raise ValueError(f"Fragment Multiplicity must be positive: {v}")
         return v
 
     @validator("molecular_multiplicity")
     def _int_if_possible(cls, v, values, **kwargs):
+        if v.is_integer():
+            # preserve existing hashes
+            v = int(v)
         if v < 1.0:
             raise ValueError("Molecular Multiplicity must be positive")
         return v
@@ -502,7 +506,7 @@ class Molecule(ProtoModel):
         return fragment_charges
 
     @property
-    def fragment_multiplicities(self) -> List[int]:
+    def fragment_multiplicities(self) -> List[float]:
         fragment_multiplicities = self.__dict__.get("fragment_multiplicities_")
         if fragment_multiplicities is None:
             fragment_multiplicities = [self.molecular_multiplicity]
@@ -803,9 +807,7 @@ class Molecule(ProtoModel):
             data = getattr(self, field)
             if field == "geometry":
                 data = float_prep(data, GEOMETRY_NOISE)
-            elif field == "fragment_charges":
-                data = float_prep(data, CHARGE_NOISE)
-            elif field == "molecular_charge":
+            elif field in ["fragment_charges", "molecular_charge", "fragment_multiplicities", "molecular_multiplicity"]:
                 data = float_prep(data, CHARGE_NOISE)
             elif field == "masses":
                 data = float_prep(data, MASS_NOISE)
diff --git a/qcelemental/molparse/chgmult.py b/qcelemental/molparse/chgmult.py
index a311d08..493335c 100644
--- a/qcelemental/molparse/chgmult.py
+++ b/qcelemental/molparse/chgmult.py
@@ -19,7 +19,7 @@ def _high_spin_sum(mult_list):
 
 
 def _mult_ok(m):
-    return isinstance(m, (int, np.integer)) and m >= 1
+    return isinstance(m, (int, np.integer, float, np.float64)) and m >= 1
 
 
 def _sufficient_electrons_for_mult(z, c, m):
@@ -430,7 +430,16 @@ def validate_and_fill_chgmult(
     if molecular_multiplicity is None:  # unneeded, but shortens the exact lists
         frag_mult_hi = _high_spin_sum(_apply_default(fragment_multiplicities, 2))
         frag_mult_lo = _high_spin_sum(_apply_default(fragment_multiplicities, 1))
-        for m in range(frag_mult_lo, frag_mult_hi + 1):
+        try:
+            mult_range = range(frag_mult_lo, frag_mult_hi + 1)
+        except TypeError:
+            if frag_mult_lo == frag_mult_hi:
+                mult_range = [frag_mult_hi]
+            else:
+                raise ValidationError(
+                    f"Cannot process: please fully specify float multiplicity: m: {molecular_multiplicity} fm: {fragment_multiplicities}"
+                )
+        for m in mult_range:
             cgmp_exact_m.append(m)
 
     #   * (S6) suggest range of missing mult = tot - high_spin_sum(frag - 1),
@@ -450,7 +459,16 @@ def validate_and_fill_chgmult(
 
     for ifr in range(nfr):
         if fragment_multiplicities[ifr] is None:  # unneeded, but shortens the exact lists
-            for m in reversed(range(max(missing_mult_lo, 1), missing_mult_hi + 1)):
+            try:
+                mult_range = reversed(range(max(missing_mult_lo, 1), missing_mult_hi + 1))
+            except TypeError:
+                if missing_mult_lo == missing_mult_hi:
+                    mult_range = [missing_mult_hi]
+                else:
+                    raise ValidationError(
+                        f"Cannot process: please fully specify float multiplicity: m: {molecular_multiplicity} fm: {fragment_multiplicities}"
+                    )
+            for m in mult_range:
                 cgmp_exact_fm[ifr].append(m)
             cgmp_exact_fm[ifr].append(1)
             cgmp_exact_fm[ifr].append(2)


Test patch:
diff --git a/qcelemental/tests/test_molecule.py b/qcelemental/tests/test_molecule.py
index 46d7ced..b2154c0 100644
--- a/qcelemental/tests/test_molecule.py
+++ b/qcelemental/tests/test_molecule.py
@@ -2,6 +2,7 @@
 Tests the imports and exports of the Molecule object.
 """
 
+
 import numpy as np
 import pytest
 
@@ -798,6 +799,15 @@ _ref_mol_multiplicity_hash = {
     "triplet": "7caca87a",
     "disinglet": "83a85546",
     "ditriplet": "71d6ba82",
+    # float mult
+    "singlet_point1": "4e9e2587",
+    "singlet_epsilon": "ad3f5fab",
+    "triplet_point1": "ad35cc28",
+    "triplet_point1_minus": "b63d6983",
+    "triplet_point00001": "7107b7ac",
+    "disinglet_epsilon": "fb0aaaca",
+    "ditriplet_point1": "33d47d5f",
+    "ditriplet_point00001": "7f0ac640",
 }
 
 
@@ -806,14 +816,26 @@ _ref_mol_multiplicity_hash = {
     [
         pytest.param(3, 3, False, "triplet"),
         pytest.param(3, 3, True, "triplet"),
-        # 3.1 -> 3 (validate=False) below documents the present bad behavior where a float mult
-        #   simply gets cast to int with no error. This will change soon. The validate=True throws a
-        #   nonspecific error that at least mentions type.
-        pytest.param(3.1, 3, False, "triplet"),
+        # before float multiplicity was allowed, 3.1 (below) was coerced into 3 with validate=False,
+        #   and validate=True threw a type-mentioning error. Now, 2.9 is allowed for both validate=T/F
+        pytest.param(3.1, 3.1, False, "triplet_point1"),
+        # validate=True counterpart fails b/c insufficient electrons in He for more than triplet
+        pytest.param(2.9, 2.9, False, "triplet_point1_minus"),
+        pytest.param(2.9, 2.9, True, "triplet_point1_minus"),
+        pytest.param(3.00001, 3.00001, False, "triplet_point00001"),
+        # validate=True counterpart fails like 3.1 above
+        pytest.param(2.99999, 2.99999, False, "triplet_point00001"),  # hash agrees w/3.00001 above b/c <CHARGE_NOISE
+        pytest.param(2.99999, 2.99999, True, "triplet_point00001"),
         pytest.param(3.0, 3, False, "triplet"),
         pytest.param(3.0, 3, True, "triplet"),
         pytest.param(1, 1, False, "singlet"),
         pytest.param(1, 1, True, "singlet"),
+        pytest.param(1.000000000000000000002, 1, False, "singlet"),
+        pytest.param(1.000000000000000000002, 1, True, "singlet"),
+        pytest.param(1.000000000000002, 1.000000000000002, False, "singlet_epsilon"),
+        pytest.param(1.000000000000002, 1.000000000000002, True, "singlet_epsilon"),
+        pytest.param(1.1, 1.1, False, "singlet_point1"),
+        pytest.param(1.1, 1.1, True, "singlet_point1"),
         pytest.param(None, 1, False, "singlet"),
         pytest.param(None, 1, True, "singlet"),
         # fmt: off
@@ -841,6 +863,9 @@ def test_mol_multiplicity_types(mult_in, mult_store, validate, exp_hash):
     [
         pytest.param(-3, False, "Multiplicity must be positive"),
         pytest.param(-3, True, "Multiplicity must be positive"),
+        pytest.param(0.9, False, "Multiplicity must be positive"),
+        pytest.param(0.9, True, "Multiplicity must be positive"),
+        pytest.param(3.1, True, "Inconsistent or unspecified chg/mult"),  # insufficient electrons in He
     ],
 )
 def test_mol_multiplicity_types_errors(mult_in, validate, error):
@@ -859,10 +884,11 @@ def test_mol_multiplicity_types_errors(mult_in, validate, error):
     [
         pytest.param(5, [3, 3], [3, 3], False, "ditriplet"),
         pytest.param(5, [3, 3], [3, 3], True, "ditriplet"),
-        # 3.1 -> 3 (validate=False) below documents the present bad behavior where a float mult
-        #   simply gets cast to int with no error. This will change soon. The validate=True throws a
-        #   irreconcilable error.
-        pytest.param(5, [3.1, 3.4], [3, 3], False, "ditriplet"),
+        # before float multiplicity was allowed, [3.1, 3.4] (below) were coerced into [3, 3] with validate=False.
+        #   Now, [2.9, 2.9] is allowed for both validate=T/F.
+        pytest.param(5, [3.1, 3.4], [3.1, 3.4], False, "ditriplet_point1"),
+        pytest.param(5, [2.99999, 3.00001], [2.99999, 3.00001], False, "ditriplet_point00001"),
+        pytest.param(5, [2.99999, 3.00001], [2.99999, 3.00001], True, "ditriplet_point00001"),
         # fmt: off
         pytest.param(5, [3.0, 3.], [3, 3], False, "ditriplet"),
         pytest.param(5, [3.0, 3.], [3, 3], True, "ditriplet"),
@@ -871,6 +897,18 @@ def test_mol_multiplicity_types_errors(mult_in, validate, error):
         pytest.param(1, [1, 1], [1, 1], True, "disinglet"),
         # None in frag_mult not allowed for validate=False
         pytest.param(1, [None, None], [1, 1], True, "disinglet"),
+        pytest.param(1, [1.000000000000000000002, 0.999999999999999999998], [1, 1], False, "disinglet"),
+        pytest.param(1, [1.000000000000000000002, 0.999999999999999999998], [1, 1], True, "disinglet"),
+        pytest.param(
+            1,
+            [1.000000000000002, 1.000000000000004],
+            [1.000000000000002, 1.000000000000004],
+            False,
+            "disinglet_epsilon",
+        ),
+        pytest.param(
+            1, [1.000000000000002, 1.000000000000004], [1.000000000000002, 1.000000000000004], True, "disinglet_epsilon"
+        ),
     ],
 )
 def test_frag_multiplicity_types(mol_mult_in, mult_in, mult_store, validate, exp_hash):
@@ -902,6 +940,9 @@ def test_frag_multiplicity_types(mol_mult_in, mult_in, mult_store, validate, exp
     [
         pytest.param([-3, 1], False, "Multiplicity must be positive"),
         pytest.param([-3, 1], True, "Multiplicity must be positive"),
+        pytest.param(
+            [3.1, 3.4], True, "Inconsistent or unspecified chg/mult"
+        ),  # insufficient e- for triplet+ on He in frag 1
     ],
 )
 def test_frag_multiplicity_types_errors(mult_in, validate, error):
diff --git a/qcelemental/tests/test_molparse_validate_and_fill_chgmult.py b/qcelemental/tests/test_molparse_validate_and_fill_chgmult.py
index 0c18649..0f85bb0 100644
--- a/qcelemental/tests/test_molparse_validate_and_fill_chgmult.py
+++ b/qcelemental/tests/test_molparse_validate_and_fill_chgmult.py
@@ -94,6 +94,12 @@ working_chgmults = [
         (-2.4, [-2.4, 0, 0], 3, [1, 2, 2]),
         "a83a3356",
     ),  # 166
+    (("He", None, [None], 2.8, [None]), (0, [0], 2.8, [2.8]), "3e10e7b5"),  # 180
+    (("He", None, [None], None, [2.8]), (0, [0], 2.8, [2.8]), "3e10e7b5"),  # 181
+    (("N/N/N", None, [None, None, None], 2.2, [2, 2, 2.2]), (0, [0, 0, 0], 2.2, [2, 2, 2.2]), "798ee5d4"),  # 183
+    (("N/N/N", None, [None, None, None], 4.2, [2, 2, 2.2]), (0, [0, 0, 0], 4.2, [2, 2, 2.2]), "ed6d1f35"),  # 185
+    (("N/N/N", None, [None, None, None], None, [2, 2, 2.2]), (0, [0, 0, 0], 4.2, [2, 2, 2.2]), "ed6d1f35"),  # 186
+    (("N/N/N", None, [2, -2, None], 2.2, [2, 2, 2.2]), (0, [2, -2, 0], 2.2, [2, 2, 2.2]), "66e655c0"),  # 187
 ]
 
 
@@ -153,6 +159,8 @@ def test_validate_and_fill_chgmult_mol_model(systemtranslator, inp, expected, ex
         ("Gh", None, [None], 3, [None]),  # 60
         ("Gh/He", None, [2, None], None, [None, None]),  # 62
         ("Gh/Ne", 2, [-2, None], None, [None, None]),  # 65b
+        ("He", None, [None], 3.2, [None]),  # 182
+        ("N/N/N", None, [None, None, None], 2.2, [None, None, 2.2]),  # 184
     ],
 )
 def test_validate_and_fill_chgmult_irreconcilable(systemtranslator, inp):
@@ -173,6 +181,8 @@ def test_validate_and_fill_chgmult_irreconcilable(systemtranslator, inp):
 # 35 - insufficient electrons
 # 55 - both (1, (1, 0.0, 0.0), 4, (1, 3, 2)) and (1, (0.0, 0.0, 1), 4, (2, 3, 1)) plausible
 # 65 - non-0/1 on Gh fragment errors normally but reset by zero_ghost_fragments
+# 182 - insufficient electrons on He
+# 184 - decline to guess fragment multiplicities when floats involved
 
 
 @pytest.fixture