Raise ValueError when setting chem potentials with mixed type but species duplicates

CHANGES.md

@@ -55,6 +55,9 @@ Use this section to keep track of changes in the works.
 ([lbluque](https://github.com/lbluque))
 
 ### Fixed
+* Fixed loading `ClusterSubspace` with polynomial basis from dict.
+[\#112](https://github.com/CederGroupHub/smol/pull/112)
+  ([lbluque](https://github.com/lbluque))
 * Fixed `Sublattice` serialization, saving/loading `SiteSpaces`.
 [\#96](https://github.com/CederGroupHub/smol/pull/96)
   ([lbluque](https://github.com/lbluque))

examples/1-creating-a-ce.ipynb

@@ -126,7 +126,7 @@
       "      [Orbit] id: 7  orderings: 1   multiplicity: 6    no. symops: 2   \n",
       "              [Base Cluster] Radius: 1.48  Centroid: [0.   0.33 0.67]   Points: [[ 0.   0.   1. ]  [-0.5  0.5  0.5]  [ 0.5  0.5  0.5]]         \n",
       "      [Orbit] id: 8  orderings: 1   multiplicity: 6    no. symops: 2   \n",
-      "              [Base Cluster] Radius: 1.48  Centroid: [0.5  0.5  0.83]   Points: [[1.  0.  1. ]  [0.5 0.5 0.5]  [0.  1.  1. ]]                  \n",
+      "              [Base Cluster] Radius: 1.48  Centroid: [0.5  0.17 0.83]   Points: [[1.  0.  1. ]  [0.5 0.5 0.5]  [0.  0.  1. ]]                  \n",
       "      [Orbit] id: 9  orderings: 1   multiplicity: 2    no. symops: 6   \n",
       "              [Base Cluster] Radius: 1.48  Centroid: [0.33 0.33 0.  ]   Points: [[1. 0. 0.]  [0. 1. 0.]  [0. 0. 0.]]                           \n",
       "      [Orbit] id: 10 orderings: 1   multiplicity: 2    no. symops: 6   \n",
@@ -237,12 +237,12 @@
     "\n",
     "In `smol` the coefficients from the fit are not exactly the ECI's but the ECI times the multiplicity of their orbit.\n",
     "\n",
-    "Currently we also have the old l1regs estimator from pyabinitio in `smol.learn` as `WDRLasso`, but it will likely be deprecated and removed at some point..."
+    "Currently we also have the old l1regs estimator from pyabinitio in `theorytoolkit.regression` as `WDRLasso`, but it will likely be deprecated and removed at some point..."
    ]
   },
   {
    "cell_type": "code",
-   "execution_count": 7,
+   "execution_count": 8,
    "metadata": {},
    "outputs": [],
    "source": [
@@ -266,7 +266,7 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 8,
+   "execution_count": 9,
    "metadata": {},
    "outputs": [
     {
@@ -303,14 +303,14 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 9,
+   "execution_count": 10,
    "metadata": {},
    "outputs": [
     {
      "name": "stdout",
      "output_type": "stream",
      "text": [
-      "The predicted energy for a structure with composition Li+3 Ni3+3 Ni4+3 O2-12 is -34.57411118658173 eV/prim.\n",
+      "The predicted energy for a structure with composition Li+3 Ni4+3 Ni3+3 O2-12 is -34.504959637039185 eV/prim.\n",
       "\n",
       "The fitted coefficients are:\n",
       "[-3.44424307e+01  1.52944807e+00  1.52944807e+00 -7.11937730e-02\n",
@@ -324,7 +324,6 @@
       "\n",
       "ClusterExpansion:\n",
       "    Prim Composition: Li+0.5 Ni3+0.5 Ni4+0.5 O2-2\n",
-      " Num fit structures: 27\n",
       "Num corr functions: 11\n",
       "    [Orbit]  id: 0  \n",
       "        bit       eci\n",
@@ -366,7 +365,7 @@
      "name": "stderr",
      "output_type": "stream",
      "text": [
-      "<ipython-input-9-ccef1ce499e4>:5: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray\n",
+      "<ipython-input-10-ccef1ce499e4>:5: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray\n",
       "  structure = np.random.choice(wrangler.structures)\n"
      ]
     }
@@ -398,7 +397,7 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 10,
+   "execution_count": 11,
    "metadata": {},
    "outputs": [],
    "source": [
@@ -419,7 +418,7 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 11,
+   "execution_count": 12,
    "metadata": {},
    "outputs": [
     {

examples/2-running-canonical-mc.ipynb

@@ -112,7 +112,7 @@
      "name": "stdout",
      "output_type": "stream",
      "text": [
-      "Sampling information: {'name': 'CanonicalEnsemble', 'kernel': 'Metropolis', 'step': 'swap', 'seed': 1999690849404472642}\n"
+      "Sampling information: {'name': 'CanonicalEnsemble', 'kernel': 'Metropolis', 'step': 'swap', 'seed': 16199914525253226741}\n"
      ]
     }
    ],

smol/moca/ensemble/semigrand.py

@@ -12,6 +12,7 @@
 
 from abc import abstractmethod
 from math import log
+from collections import Counter
 import numpy as np
 
 from monty.json import MSONable
@@ -141,6 +142,14 @@ def chemical_potentials(self):
     @chemical_potentials.setter
     def chemical_potentials(self, value):
         """Set the chemical potentials and update table."""
+        for sp, count in Counter(map(get_species, value.keys())).items():
+            if count > 1:
+                raise ValueError(
+                    f"{count} values of the chemical potential for the same "
+                    f"species {sp} were provided.\n Make sure the dictionary "
+                    "you are using has only string keys or only Species "
+                    "objects as keys."
+                )
         value = {get_species(k): v for k, v in value.items()}
         if set(value.keys()) != set(self._mus.keys()):
             raise ValueError('Chemical potentials given are missing species. '
@@ -294,6 +303,16 @@ def fugacity_fractions(self):
     @fugacity_fractions.setter
     def fugacity_fractions(self, value):
         """Set the fugacity fractions and update table."""
+        for sub in value:
+            for sp, count in Counter(map(get_species, sub.keys())).items():
+                if count > 1:
+                    raise ValueError(
+                        f"{count} values of the fugacity for the same "
+                        f"species {sp} were provided.\n Make sure the "
+                        "dictionaries you are using have only "
+                        "string keys or only Species objects as keys."
+                    )
+
         value = [{get_species(k): v for k, v in sub.items()} for sub in value]
         if not all(sum(fus.values()) == 1 for fus in value):
             raise ValueError('Fugacity ratios must add to one.')

tests/test_moca/test_ensemble.py

@@ -38,8 +38,8 @@ def canonical_ensemble(composite_processor):
 
 @pytest.fixture
 def mugrand_ensemble(composite_processor):
-    chemical_potentials= {sp: 0.3 for space in composite_processor.unique_site_spaces
-                          for sp in space.keys()}
+    chemical_potentials = {sp: 0.3 for space in composite_processor.unique_site_spaces
+                           for sp in space.keys()}
     return MuSemiGrandEnsemble(composite_processor,
                                chemical_potentials=chemical_potentials)
 
@@ -147,6 +147,7 @@ def test_compute_feature_vector(mugrand_ensemble):
 def test_bad_chemical_potentials(mugrand_ensemble):
     proc = mugrand_ensemble.processor
     chem_pots = mugrand_ensemble.chemical_potentials
+    print(chem_pots)
     with pytest.raises(ValueError):
         items = list(chem_pots.items())
         mugrand_ensemble.chemical_potentials = {items[0][0]: items[0][1]}
@@ -159,7 +160,10 @@ def test_bad_chemical_potentials(mugrand_ensemble):
         del chem_pots['foo']
         chem_pots.pop(items[0][0])
         MuSemiGrandEnsemble(proc, chemical_potentials=chem_pots)
-
+    with pytest.raises(ValueError):
+        chem_pots[str(list(chem_pots.keys()))[0]] = 0.0
+        mugrand_ensemble.chemical_potentials = chem_pots
+
 
 def test_build_mu_table(mugrand_ensemble):
     proc = mugrand_ensemble.processor
@@ -213,6 +217,9 @@ def test_bad_fugacity_fractions(fugrand_ensemble):
     with pytest.raises(ValueError):
         del fug_fracs[0]['foo']
         FuSemiGrandEnsemble(proc, fugacity_fractions=fug_fracs)
+    with pytest.raises(ValueError):
+        fug_fracs[0][str(list(fug_fracs[0].keys())[0])] = 0.0
+        fugrand_ensemble.fugacity_fractions = fug_fracs
 
 
 def test_build_fu_table(fugrand_ensemble):