Repository: simpeg/simpeg
Pull request #1376 (2024-03-08 18:37:31)

Remove the cell_weights attribute in regularizations
<!--
Thanks for contributing a pull request to SimPEG!
Remember to use a personal fork of SimPEG to propose changes.

Check out the stages of a pull request at
https://docs.simpeg.xyz/content/getting_started/contributing/pull-requests.html

Note that we are a team of volunteers and we appreciate your
patience during the review process.

Again, thanks for contributing!

Feel free to remove lines from this template that do not apply to you pull request.
-->

#### Summary
<!-- Add a summary of this Pull Request -->

Add test checking errors are raised after accessing the property or passing `cell_weights` argument to regularization's constructor. Replace usage of `cell_weights` in examples and code.

#### PR Checklist
* [ ] If this is a work in progress PR, set as a Draft PR
* [ ] Linted my code according to the [style guides](https://docs.simpeg.xyz/content/getting_started/contributing/code-style.html).
* [ ] Added [tests](https://docs.simpeg.xyz/content/getting_started/practices.html#testing) to verify changes to the code.
* [ ] Added necessary documentation to any new functions/classes following the
      expect [style](https://docs.simpeg.xyz/content/getting_started/practices.html#documentation).
* [ ] Marked as ready for review (if this is was a draft PR), and converted
      to a Pull Request
* [ ] Tagged ``@simpeg/simpeg-developers`` when ready for review.

#### Reference issue
<!--Example: write "Closes #NNNN" to automatically close that issue on merge.-->

#### What does this implement/fix?
<!--Please explain your changes.-->

#### Additional information
<!--Any additional information you think is important.-->


<!--
Once all tests pass and the code has been reviewed and approved, it will be merged into main
-->


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/simpeg/simpeg/pull/1376?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) Report
Attention: Patch coverage is `57.14286%` with `3 lines` in your changes are missing coverage. Please review.
> Project coverage is 46.19%. Comparing base [(`7fe2c33`)](https://app.codecov.io/gh/simpeg/simpeg/commit/7fe2c33736e9632fd9c36296d0f89e32a53dfea5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) to head [(`a721763`)](https://app.codecov.io/gh/simpeg/simpeg/pull/1376?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg).

| [Files](https://app.codecov.io/gh/simpeg/simpeg/pull/1376?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) | Patch % | Lines |
|---|---|---|
| [SimPEG/regularization/base.py](https://app.codecov.io/gh/simpeg/simpeg/pull/1376?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg#diff-U2ltUEVHL3JlZ3VsYXJpemF0aW9uL2Jhc2UucHk=) | 57.14% | [3 Missing :warning: ](https://app.codecov.io/gh/simpeg/simpeg/pull/1376?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) |

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##             main    #1376       +/-   ##
===========================================
- Coverage   82.30%   46.19%   -36.12%     
===========================================
  Files         165      165               
  Lines       25240    25236        -4     
===========================================
- Hits        20774    11657     -9117     
- Misses       4466    13579     +9113     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/simpeg/simpeg/pull/1376?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg).

santisoler: @jcapriot would you take a look at this. I think is ready.

Patch:
diff --git a/SimPEG/electromagnetics/static/induced_polarization/run.py b/SimPEG/electromagnetics/static/induced_polarization/run.py
index 53fbd717f..a7372118b 100644
--- a/SimPEG/electromagnetics/static/induced_polarization/run.py
+++ b/SimPEG/electromagnetics/static/induced_polarization/run.py
@@ -47,7 +47,6 @@ def run_inversion(
             mesh,
             active_cells=actind,
             mapping=regmap,
-            cell_weights=mesh.cell_volumes[actind],
         )
         reg.alpha_s = alpha_s
         reg.alpha_x = alpha_x
diff --git a/SimPEG/regularization/base.py b/SimPEG/regularization/base.py
index ca38be2e3..5aaeaedfd 100644
--- a/SimPEG/regularization/base.py
+++ b/SimPEG/regularization/base.py
@@ -1,5 +1,4 @@
 from __future__ import annotations
-import warnings
 
 import numpy as np
 from discretize.base import BaseMesh
@@ -75,18 +74,9 @@ class BaseRegularization(BaseObjectiveFunction):
                 "Please use 'active_cells' instead."
             )
         if (key := "cell_weights") in kwargs:
-            if weights is not None:
-                raise ValueError(
-                    f"Cannot simultaneously pass 'weights' and '{key}'. "
-                    "Pass 'weights' only."
-                )
-            warnings.warn(
-                f"The '{key}' argument has been deprecated, please use 'weights'. "
-                "It will be removed in future versions of SimPEG.",
-                DeprecationWarning,
-                stacklevel=2,
+            raise TypeError(
+                f"'{key}' argument has been removed. Please use 'weights' instead."
             )
-            weights = kwargs.pop(key)
 
         super().__init__(nP=None, mapping=None, **kwargs)
         self._regularization_mesh = mesh
@@ -302,23 +292,19 @@ class BaseRegularization(BaseObjectiveFunction):
     @property
     def cell_weights(self) -> np.ndarray:
         """Deprecated property for 'volume' and user defined weights."""
-        warnings.warn(
-            "cell_weights are deprecated please access weights using the `set_weights`,"
-            " `get_weights`, and `remove_weights` functionality. This will be removed in 0.19.0",
-            FutureWarning,
-            stacklevel=2,
+        raise AttributeError(
+            "'cell_weights' has been removed. "
+            "Please access weights using the `set_weights`, `get_weights`, and "
+            "`remove_weights` methods."
         )
-        return np.prod(list(self._weights.values()), axis=0)
 
     @cell_weights.setter
     def cell_weights(self, value):
-        warnings.warn(
-            "cell_weights are deprecated please access weights using the `set_weights`,"
-            " `get_weights`, and `remove_weights` functionality. This will be removed in 0.19.0",
-            FutureWarning,
-            stacklevel=2,
+        raise AttributeError(
+            "'cell_weights' has been removed. "
+            "Please access weights using the `set_weights`, `get_weights`, and "
+            "`remove_weights` methods."
         )
-        self.set_weights(cell_weights=value)
 
     def get_weights(self, key) -> np.ndarray:
         """Cell weights for a given key.
@@ -1583,6 +1569,11 @@ class WeightedLeastSquares(ComboObjectiveFunction):
                 "Please use 'active_cells' instead."
             )
 
+        if (key := "cell_weights") in kwargs:
+            raise TypeError(
+                f"'{key}' argument has been removed. Please use 'weights' instead."
+            )
+
         self.alpha_s = alpha_s
         if alpha_x is not None:
             if length_scale_x is not None:
@@ -1714,20 +1705,19 @@ class WeightedLeastSquares(ComboObjectiveFunction):
 
     @property
     def cell_weights(self):
-        # All of the objective functions should have the same weights,
-        # so just grab the one from smallness here, which should also
-        # trigger the deprecation warning
-        return self.objfcts[0].cell_weights
+        raise AttributeError(
+            "'cell_weights' has been removed. "
+            "Please access weights using the `set_weights`, `get_weights`, and "
+            "`remove_weights` methods."
+        )
 
     @cell_weights.setter
     def cell_weights(self, value):
-        warnings.warn(
-            "cell_weights are deprecated please access weights using the `set_weights`,"
-            " `get_weights`, and `remove_weights` functionality. This will be removed in 0.19.0",
-            FutureWarning,
-            stacklevel=2,
+        raise AttributeError(
+            "'cell_weights' has been removed. "
+            "Please access weights using the `set_weights`, `get_weights`, and "
+            "`remove_weights` methods."
         )
-        self.set_weights(cell_weights=value)
 
     @property
     def alpha_s(self):
diff --git a/examples/01-maps/plot_sumMap.py b/examples/01-maps/plot_sumMap.py
index dd7a1d012..d86976ea0 100644
--- a/examples/01-maps/plot_sumMap.py
+++ b/examples/01-maps/plot_sumMap.py
@@ -138,7 +138,7 @@ def run(plotIt=True):
     regMesh = TensorMesh([len(domains)])
 
     reg_m1 = regularization.Sparse(regMesh, mapping=wires.homo)
-    reg_m1.cell_weights = wires.homo * wr
+    reg_m1.set_weights(cell_weights=wires.homo * wr)
     reg_m1.norms = [0, 2]
     reg_m1.mref = np.zeros(sumMap.shape[1])
 
@@ -146,7 +146,7 @@ def run(plotIt=True):
     reg_m2 = regularization.Sparse(
         mesh, active_cells=actv, mapping=wires.hetero, gradient_type="components"
     )
-    reg_m2.cell_weights = wires.hetero * wr
+    reg_m2.set_weights(cell_weights=wires.hetero * wr)
     reg_m2.norms = [0, 0, 0, 0]
     reg_m2.mref = np.zeros(sumMap.shape[1])
 
diff --git a/examples/08-vrm/plot_inv_vrm_eq.py b/examples/08-vrm/plot_inv_vrm_eq.py
index aba2d0fde..e0eee4ff6 100644
--- a/examples/08-vrm/plot_inv_vrm_eq.py
+++ b/examples/08-vrm/plot_inv_vrm_eq.py
@@ -196,7 +196,7 @@ w = utils.mkvc((np.sum(np.array(problem_inv.A) ** 2, axis=0))) ** 0.5
 w = w / np.max(w)
 w = w
 
-reg = regularization.Smallness(mesh=mesh, active_cells=actCells, cell_weights=w)
+reg = regularization.Smallness(mesh=mesh, active_cells=actCells, weights=w)
 opt = optimization.ProjectedGNCG(
     maxIter=20, lower=0.0, upper=1e-2, maxIterLS=20, tolCG=1e-4
 )
diff --git a/examples/10-pgi/plot_inv_1_PGI_Linear_1D_joint_WithRelationships.py b/examples/10-pgi/plot_inv_1_PGI_Linear_1D_joint_WithRelationships.py
index 5da593295..adabf3f35 100644
--- a/examples/10-pgi/plot_inv_1_PGI_Linear_1D_joint_WithRelationships.py
+++ b/examples/10-pgi/plot_inv_1_PGI_Linear_1D_joint_WithRelationships.py
@@ -235,11 +235,11 @@ mcluster_no_map = inv.run(minit)
 reg1 = regularization.WeightedLeastSquares(
     mesh, alpha_s=1.0, alpha_x=1.0, mapping=wires.m1
 )
-reg1.cell_weights = wr1
+reg1.set_weights(cell_weights=wr1)
 reg2 = regularization.WeightedLeastSquares(
     mesh, alpha_s=1.0, alpha_x=1.0, mapping=wires.m2
 )
-reg2.cell_weights = wr2
+reg2.set_weights(cell_weights=wr2)
 reg = reg1 + reg2
 
 opt = optimization.ProjectedGNCG(


Test patch:
diff --git a/tests/base/test_directives.py b/tests/base/test_directives.py
index 0d2f2f105..2af5c58e3 100644
--- a/tests/base/test_directives.py
+++ b/tests/base/test_directives.py
@@ -161,7 +161,11 @@ class ValidationInInversion(unittest.TestCase):
         test_directive.update()
 
         for reg_i in reg.objfcts:
-            self.assertTrue(np.all(np.isclose(test_weights, reg_i.cell_weights)))
+            # Get all weights in regularization
+            weights = [reg_i.get_weights(key) for key in reg_i.weights_keys]
+            # Compute the product of all weights
+            weights = np.prod(weights, axis=0)
+            self.assertTrue(np.all(np.isclose(test_weights, weights)))
             reg_i.remove_weights("sensitivity")
 
         # self.test_sensitivity_weighting_subroutine(test_weights, test_directive)
@@ -201,7 +205,11 @@ class ValidationInInversion(unittest.TestCase):
         test_directive.update()
 
         for reg_i in reg.objfcts:
-            self.assertTrue(np.all(np.isclose(test_weights, reg_i.cell_weights)))
+            # Get all weights in regularization
+            weights = [reg_i.get_weights(key) for key in reg_i.weights_keys]
+            # Compute the product of all weights
+            weights = np.prod(weights, axis=0)
+            self.assertTrue(np.all(np.isclose(test_weights, weights)))
             reg_i.remove_weights("sensitivity")
 
         # self.test_sensitivity_weighting_subroutine(test_weights, test_directive)
@@ -241,7 +249,11 @@ class ValidationInInversion(unittest.TestCase):
         test_directive.update()
 
         for reg_i in reg.objfcts:
-            self.assertTrue(np.all(np.isclose(test_weights, reg_i.cell_weights)))
+            # Get all weights in regularization
+            weights = [reg_i.get_weights(key) for key in reg_i.weights_keys]
+            # Compute the product of all weights
+            weights = np.prod(weights, axis=0)
+            self.assertTrue(np.all(np.isclose(test_weights, weights)))
             reg_i.remove_weights("sensitivity")
 
         # self.test_sensitivity_weighting_subroutine(test_weights, test_directive)
diff --git a/tests/base/test_regularization.py b/tests/base/test_regularization.py
index 207e8d9cc..c50a4a83a 100644
--- a/tests/base/test_regularization.py
+++ b/tests/base/test_regularization.py
@@ -758,44 +758,13 @@ class TestWeightsKeys:
             assert reg.weights_keys == ["dummy_weight", "other_weights", "volume"]
 
 
-class TestDeprecatedArguments:
-    """
-    Test errors after simultaneously passing new and deprecated arguments.
-
-    Within these arguments are:
-
-    * ``cell_weights`` (replaced by ``weights``)
-
-    """
-
-    @pytest.fixture(params=["1D", "2D", "3D"])
-    def mesh(self, request):
-        """Sample mesh."""
-        if request.param == "1D":
-            hx = np.random.rand(10)
-            h = [hx / hx.sum()]
-        elif request.param == "2D":
-            hx, hy = np.random.rand(10), np.random.rand(9)
-            h = [h_i / h_i.sum() for h_i in (hx, hy)]
-        elif request.param == "3D":
-            hx, hy, hz = np.random.rand(10), np.random.rand(9), np.random.rand(8)
-            h = [h_i / h_i.sum() for h_i in (hx, hy, hz)]
-        return discretize.TensorMesh(h)
-
-    def test_weights(self, mesh):
-        """Test cell_weights and weights."""
-        weights = np.ones(len(mesh))
-        msg = "Cannot simultaneously pass 'weights' and 'cell_weights'."
-        with pytest.raises(ValueError, match=msg):
-            BaseRegularization(mesh, weights=weights, cell_weights=weights)
-
-
 class TestRemovedObjects:
     """
     Test if errors are raised after passing removed arguments or trying to
     access removed properties.
 
     * ``indActive`` (replaced by ``active_cells``)
+    * ``cell_weights`` (replaced by ``weights``)
 
     """
 
@@ -839,6 +808,46 @@ class TestRemovedObjects:
         with pytest.raises(NotImplementedError, match=msg):
             reg.indActive
 
+    @pytest.mark.parametrize(
+        "regularization_class",
+        (BaseRegularization, WeightedLeastSquares),
+    )
+    def test_cell_weights_argument(self, mesh, regularization_class):
+        """Test if error is raised when passing the cell_weights argument."""
+        weights = np.ones(len(mesh))
+        msg = "'cell_weights' argument has been removed. Please use 'weights' instead."
+        with pytest.raises(TypeError, match=msg):
+            regularization_class(mesh, cell_weights=weights)
+
+    @pytest.mark.parametrize(
+        "regularization_class", (BaseRegularization, WeightedLeastSquares)
+    )
+    def test_cell_weights_property(self, mesh, regularization_class):
+        """Test if error is raised when trying to access the cell_weights property."""
+        weights = {"weights": np.ones(len(mesh))}
+        msg = (
+            "'cell_weights' has been removed. "
+            "Please access weights using the `set_weights`, `get_weights`, and "
+            "`remove_weights` methods."
+        )
+        reg = regularization_class(mesh, weights=weights)
+        with pytest.raises(AttributeError, match=msg):
+            reg.cell_weights
+
+    @pytest.mark.parametrize(
+        "regularization_class", (BaseRegularization, WeightedLeastSquares)
+    )
+    def test_cell_weights_setter(self, mesh, regularization_class):
+        """Test if error is raised when trying to set the cell_weights property."""
+        msg = (
+            "'cell_weights' has been removed. "
+            "Please access weights using the `set_weights`, `get_weights`, and "
+            "`remove_weights` methods."
+        )
+        reg = regularization_class(mesh)
+        with pytest.raises(AttributeError, match=msg):
+            reg.cell_weights = "dummy variable"
+
 
 class TestRemovedRegularizations:
     """