Merge pull request #1924 from OceanParcels/remove_scaling_factors

Removing scaling_factors

Author: erikvansebille

docs/examples/tutorial_unitconverters.ipynb

@@ -351,41 +351,6 @@
     ")\n",
     "print(fieldset.Ustokes2[0, 0, 40, -5])"
    ]
-  },
-  {
-   "attachments": {},
-   "cell_type": "markdown",
-   "metadata": {},
-   "source": [
-    "## Using velocities in units other than m/s\n"
-   ]
-  },
-  {
-   "attachments": {},
-   "cell_type": "markdown",
-   "metadata": {},
-   "source": [
-    "Some OGCM store velocity data in units of e.g. cm/s. For these cases, Field objects have a method `set_scaling_factor()`.\n",
-    "\n",
-    "If your data is in cm/s and if you want to use the built-in Advection kernels, you will therefore have to use `fieldset.U.set_scaling_factor(100)` and `fieldset.V.set_scaling_factor(100)`.\n"
-   ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": null,
-   "metadata": {},
-   "outputs": [],
-   "source": [
-    "fieldset.add_field(\n",
-    "    parcels.Field(\n",
-    "        name=\"Ucm\",\n",
-    "        data=0.01 * np.ones((ydim, xdim), dtype=np.float32),\n",
-    "        grid=fieldset.U.grid,\n",
-    "    )\n",
-    ")\n",
-    "fieldset.Ucm.set_scaling_factor(100)\n",
-    "print(fieldset.Ucm[0, 0, 40, -5])"
-   ]
   }
  ],
  "metadata": {

parcels/field.py

@@ -219,7 +219,6 @@ def __init__(
         # Hack around the fact that NaN and ridiculously large values
         # propagate in SciPy's interpolators
         self.data[np.isnan(self.data)] = 0.0
-        self._scaling_factor = None
 
         self._dimensions = kwargs.pop("dimensions", None)
         self._dataFiles = kwargs.pop("dataFiles", None)
@@ -575,26 +574,6 @@ def _reshape(self, data):
 
         return data
 
-    def set_scaling_factor(self, factor):
-        """Scales the field data by some constant factor.
-
-        Parameters
-        ----------
-        factor :
-            scaling factor
-
-
-        Examples
-        --------
-        For usage examples see the following tutorial:
-
-        * `Unit converters <../examples/tutorial_unitconverters.ipynb>`__
-        """
-        if self._scaling_factor:
-            raise NotImplementedError(f"Scaling factor for field {self.name} already defined.")
-        self._scaling_factor = factor
-        self.data *= factor
-
     def _search_indices(self, time, z, y, x, ti, particle=None, search2D=False):
         if self.grid._gtype in [GridType.RectilinearSGrid, GridType.RectilinearZGrid]:
             return _search_indices_rectilinear(self, time, z, y, x, ti, particle=particle, search2D=search2D)
@@ -761,8 +740,6 @@ def write(self, filename, varname=None):
 
     def _rescale_and_set_minmax(self, data):
         data[np.isnan(data)] = 0
-        if self._scaling_factor:
-            data *= self._scaling_factor
         return data
 
     def ravel_index(self, zi, yi, xi):

parcels/fieldset.py

@@ -497,8 +497,6 @@ def from_nemo(
             gridindexingtype="nemo",
             **kwargs,
         )
-        if hasattr(fieldset, "W"):
-            fieldset.W.set_scaling_factor(-1.0)
         return fieldset
 
     @classmethod
@@ -814,8 +812,6 @@ def from_mom5(
             gridindexingtype="mom5",
             **kwargs,
         )
-        if hasattr(fieldset, "W"):
-            fieldset.W.set_scaling_factor(-1)
         return fieldset
 
     @classmethod
@@ -905,7 +901,7 @@ def from_b_grid_dataset(
             Default is False if dimensions includes time, else True
         tracer_interp_method : str
             Method for interpolation of tracer fields. It is recommended to use 'bgrid_tracer' (default)
-            Note that in the case of from_pop() and from_b_grid_dataset(), the velocity fields are default to 'bgrid_velocity'
+            Note that in the case of from_b_grid_dataset(), the velocity fields are default to 'bgrid_velocity'
         **kwargs :
             Keyword arguments passed to the :func:`Fieldset.from_netcdf` constructor.
         """

parcels/grid.py

@@ -88,13 +88,6 @@ def lat(self):
     def depth(self):
         return self._depth
 
-    def negate_depth(self):
-        """Method to flip the sign of the depth dimension of a Grid.
-        Note that this method does _not_ change the direction of the vertical velocity;
-        for that users need to add a fieldset.W.set_scaling_factor(-1.0)
-        """
-        self._depth = -self._depth
-
     @property
     def mesh(self):
         return self._mesh

parcels/kernel.py

@@ -16,7 +16,7 @@
     AdvectionRK4_3D_CROCO,
     AdvectionRK45,
 )
-from parcels.field import Field, VectorField
+from parcels.field import VectorField
 from parcels.grid import GridType
 from parcels.tools.statuscodes import (
     StatusCode,
@@ -207,23 +207,7 @@ def check_fieldsets_in_kernels(self, pyfunc):  # TODO v4: this can go into anoth
         This function is to be called from the derived class when setting up the 'pyfunc'.
         """
         if self.fieldset is not None:
-            if pyfunc is AdvectionRK4_3D:
-                warning = False
-                if (
-                    isinstance(self._fieldset.W, Field)
-                    and self._fieldset.W._creation_log != "from_nemo"
-                    and self._fieldset.W._scaling_factor is not None
-                    and self._fieldset.W._scaling_factor > 0
-                ):
-                    warning = True
-                if warning:
-                    warnings.warn(
-                        "Note that in AdvectionRK4_3D, vertical velocity is assumed positive towards increasing z. "
-                        "If z increases downward and w is positive upward you can re-orient it downwards by setting fieldset.W.set_scaling_factor(-1.)",
-                        KernelWarning,
-                        stacklevel=2,
-                    )
-            elif pyfunc is AdvectionAnalytical:
+            if pyfunc is AdvectionAnalytical:
                 if self.fieldset.particlefile is not None:
                     self.fieldset.particlefile._is_analytical = True
                 if self._fieldset.U.interp_method != "cgrid_velocity":

tests/test_grids.py

@@ -111,6 +111,8 @@ def test_time_format_in_grid():
         RectilinearZGrid(lon, lat, time=time)
 
 
+@pytest.mark.v4remove
+@pytest.mark.xfail(reason="negate_depth removed in v4")
 def test_negate_depth():
     depth = np.linspace(0, 5, 10, dtype=np.float32)
     fieldset = FieldSet.from_data(
@@ -975,4 +977,4 @@ def VelocityInterpolator(particle, fieldset, time):  # pragma: no cover
             if extrapolation:
                 assert np.allclose(pset.Wvel[0], 0, atol=1e-9)
             else:
-                assert np.allclose(pset.Wvel[0], -w * convfactor)
+                assert np.allclose(pset.Wvel[0], w * convfactor)

tests/tools/test_warnings.py

@@ -1,11 +1,8 @@
-import warnings
-
 import numpy as np
 import pytest
 
 from parcels import (
     AdvectionRK4,
-    AdvectionRK4_3D,
     AdvectionRK45,
     FieldSet,
     FieldSetWarning,
@@ -51,22 +48,7 @@ def test_file_warnings(tmp_zarrfile):
         pset.execute(AdvectionRK4, runtime=3, dt=1, output_file=pfile)
 
 
-@pytest.mark.v4alpha
-@pytest.mark.xfail(reason="https://github.com/OceanParcels/Parcels/pull/1908#issuecomment-2698014941")
 def test_kernel_warnings():
-    # positive scaling factor for W
-    filenames = str(TEST_DATA / "POPtestdata_time.nc")
-    variables = {"U": "U", "V": "V", "W": "W", "T": "T"}
-    dimensions = {"lon": "lon", "lat": "lat", "depth": "w_deps", "time": "time"}
-    with warnings.catch_warnings():
-        # ignore FieldSetWarnings (tested in test_fieldset_warnings)
-        warnings.simplefilter("ignore", FieldSetWarning)
-        fieldset = FieldSet.from_pop(filenames, variables, dimensions, mesh="flat")
-        fieldset.W._scaling_factor = 0.01
-        pset = ParticleSet(fieldset=fieldset, pclass=Particle, lon=[0], lat=[0], depth=[0], time=[0])
-        with pytest.warns(KernelWarning):
-            pset.execute(AdvectionRK4_3D, runtime=1, dt=1)
-
     # RK45 warnings
     lat = [0, 1, 5, 10]
     lon = [0, 1, 5, 10]