Merge pull request #2274 from andrew-s28/dt-required-arg

Require `dt` argument in ParticleSet.execute()

Author: erikvansebille

parcels/_core/particleset.py

@@ -17,7 +17,6 @@
 from parcels._core.warnings import ParticleSetWarning
 from parcels._logger import logger
 from parcels._reprs import particleset_repr
-from parcels.kernels import AdvectionRK4
 
 __all__ = ["ParticleSet"]
 
@@ -452,10 +451,10 @@ def set_variable_write_status(self, var, write_status):
 
     def execute(
         self,
-        pyfunc=AdvectionRK4,
+        pyfunc,
+        dt: datetime.timedelta | np.timedelta64,
         endtime: np.timedelta64 | np.datetime64 | None = None,
         runtime: datetime.timedelta | np.timedelta64 | None = None,
-        dt: datetime.timedelta | np.timedelta64 | None = None,
         output_file=None,
         verbose_progress=True,
     ):
@@ -469,17 +468,17 @@ def execute(
         pyfunc :
             Kernel function to execute. This can be the name of a
             defined Python function or a :class:`parcels.kernel.Kernel` object.
-            Kernels can be concatenated using the + operator (Default value = AdvectionRK4)
+            Kernels can be concatenated using the + operator.
+        dt (np.timedelta64):
+            Timestep interval (as a np.timedelta64 object) to be passed to the kernel.
+            Use a negative value for a backward-in-time simulation.
         endtime (np.datetime64 or np.timedelta64): :
             End time for the timestepping loop. If a np.timedelta64 is provided, it is interpreted as the total simulation time. In this case,
             the absolute end time is the start of the fieldset's time interval plus the np.timedelta64.
             If a datetime is provided, it is interpreted as the absolute end time of the simulation.
         runtime (np.timedelta64):
             The duration of the simuulation execution. Must be a np.timedelta64 object and is required to be set when the `fieldset.time_interval` is not defined.
             If the `fieldset.time_interval` is defined and the runtime is provided, the end time will be the start of the fieldset's time interval plus the runtime.
-        dt (np.timedelta64):
-            Timestep interval (as a np.timedelta64 object) to be passed to the kernel.
-            Use a negative value for a backward-in-time simulation. (Default value = 1 second)
         output_file :
             mod:`parcels.particlefile.ParticleFile` object for particle output (Default value = None)
         verbose_progress : bool
@@ -502,9 +501,6 @@ def execute(
             output_file.set_metadata(self.fieldset.gridset[0]._mesh)
             output_file.metadata["parcels_kernels"] = self._kernel.funcname
 
-        if dt is None:
-            dt = np.timedelta64(1, "s")
-
         try:
             dt = maybe_convert_python_timedelta_to_numpy(dt)
             assert not np.isnat(dt)

tests/test_kernel.py

@@ -30,7 +30,7 @@ def ErrorKernel(particles, fieldset):  # pragma: no cover
         particles.unknown_varname += 0.2
 
     with pytest.raises(KeyError, match="'unknown_varname'"):
-        pset.execute(ErrorKernel, runtime=np.timedelta64(2, "s"))
+        pset.execute(ErrorKernel, runtime=np.timedelta64(2, "s"), dt=np.timedelta64(1, "s"))
 
 
 def test_kernel_init(fieldset):

tests/test_particleset_execute.py

@@ -23,7 +23,7 @@
 from parcels._datasets.structured.generic import datasets as datasets_structured
 from parcels._datasets.unstructured.generic import datasets as datasets_unstructured
 from parcels.interpolators import UXPiecewiseConstantFace
-from parcels.kernels import AdvectionEE
+from parcels.kernels import AdvectionEE, AdvectionRK4
 from tests import utils
 from tests.common_kernels import DoNothing
 
@@ -61,64 +61,62 @@ def zonal_flow_fieldset() -> FieldSet:
     return FieldSet([U, V, UV])
 
 
-def test_pset_execute_implicit_dt_one_second(fieldset):
-    pset = ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle)
-    pset.execute(DoNothing, runtime=np.timedelta64(1, "s"))
-
-    time = pset.time.copy()
-
-    pset.execute(DoNothing, runtime=np.timedelta64(1, "s"))
-    np.testing.assert_array_equal(pset.time, time + np.timedelta64(1, "s"))
-
-
 def test_pset_execute_invalid_arguments(fieldset, fieldset_no_time_interval):
     for dt in [1, np.timedelta64(0, "s"), np.timedelta64(None)]:
         with pytest.raises(
             ValueError,
             match="dt must be a non-zero datetime.timedelta or np.timedelta64 object, got .*",
         ):
-            ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(dt=dt)
+            ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(AdvectionRK4, dt=dt)
 
     with pytest.raises(
         ValueError,
         match="runtime and endtime are mutually exclusive - provide one or the other. Got .*",
     ):
         ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(
-            runtime=np.timedelta64(1, "s"), endtime=np.datetime64("2100-01-01")
+            AdvectionRK4, runtime=np.timedelta64(1, "s"), endtime=np.datetime64("2100-01-01"), dt=np.timedelta64(1, "s")
         )
 
     with pytest.raises(
         ValueError,
         match="The runtime must be a datetime.timedelta or np.timedelta64 object. Got .*",
     ):
-        ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(runtime=1)
+        ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(
+            AdvectionRK4, runtime=1, dt=np.timedelta64(1, "s")
+        )
 
     msg = """Calculated/provided end time of .* is not in fieldset time interval .* Either reduce your runtime, modify your provided endtime, or change your release timing.*"""
     with pytest.raises(
         ValueError,
         match=msg,
     ):
-        ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(endtime=np.datetime64("1990-01-01"))
+        ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(
+            AdvectionRK4, endtime=np.datetime64("1990-01-01"), dt=np.timedelta64(1, "s")
+        )
 
     with pytest.raises(
         ValueError,
         match=msg,
     ):
         ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(
-            endtime=np.datetime64("2100-01-01"), dt=np.timedelta64(-1, "s")
+            AdvectionRK4, endtime=np.datetime64("2100-01-01"), dt=np.timedelta64(-1, "s")
         )
 
     with pytest.raises(
         ValueError,
         match="The endtime must be of the same type as the fieldset.time_interval start time. Got .*",
     ):
-        ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(endtime=12345)
+        ParticleSet(fieldset, lon=[0.2], lat=[5.0], pclass=Particle).execute(
+            AdvectionRK4, endtime=12345, dt=np.timedelta64(1, "s")
+        )
 
     with pytest.raises(
         ValueError,
         match="The runtime must be provided when the time_interval is not defined for a fieldset.",
     ):
-        ParticleSet(fieldset_no_time_interval, lon=[0.2], lat=[5.0], pclass=Particle).execute()
+        ParticleSet(fieldset_no_time_interval, lon=[0.2], lat=[5.0], pclass=Particle).execute(
+            AdvectionRK4, dt=np.timedelta64(1, "s")
+        )
 
 
 @pytest.mark.parametrize(
@@ -222,13 +220,12 @@ def AddLat(particles, fieldset):  # pragma: no cover
 
 @pytest.mark.parametrize(
     "starttime, endtime, dt",
-    [(0, 10, 1), (0, 10, 3), (2, 16, 3), (20, 10, -1), (20, 0, -2), (5, 15, None)],
+    [(0, 10, 1), (0, 10, 3), (2, 16, 3), (20, 10, -1), (20, 0, -2), (5, 15, 1)],
 )
 def test_execution_endtime(fieldset, starttime, endtime, dt):
     starttime = fieldset.time_interval.left + np.timedelta64(starttime, "s")
     endtime = fieldset.time_interval.left + np.timedelta64(endtime, "s")
-    if dt is not None:
-        dt = np.timedelta64(dt, "s")
+    dt = np.timedelta64(dt, "s")
     pset = ParticleSet(fieldset, time=starttime, lon=0, lat=0)
     pset.execute(DoNothing, endtime=endtime, dt=dt)
     assert abs(pset.time_nextloop - endtime) < np.timedelta64(1, "ms")
@@ -348,15 +345,14 @@ def MoveLeft(particles, fieldset):  # pragma: no cover
 
 @pytest.mark.parametrize(
     "starttime, runtime, dt",
-    [(0, 10, 1), (0, 10, 3), (2, 16, 3), (20, 10, -1), (20, 0, -2), (5, 15, None)],
+    [(0, 10, 1), (0, 10, 3), (2, 16, 3), (20, 10, -1), (20, 0, -2), (5, 15, 1)],
 )
 @pytest.mark.parametrize("npart", [1, 10])
 def test_execution_runtime(fieldset, starttime, runtime, dt, npart):
     starttime = fieldset.time_interval.left + np.timedelta64(starttime, "s")
     runtime = np.timedelta64(runtime, "s")
-    sign_dt = 1 if dt is None else np.sign(dt)
-    if dt is not None:
-        dt = np.timedelta64(dt, "s")
+    sign_dt = np.sign(dt)
+    dt = np.timedelta64(dt, "s")
     pset = ParticleSet(fieldset, time=starttime, lon=np.zeros(npart), lat=np.zeros(npart))
     pset.execute(DoNothing, runtime=runtime, dt=dt)
     assert all([abs(p.time_nextloop - starttime - runtime * sign_dt) < np.timedelta64(1, "ms") for p in pset])
@@ -475,9 +471,9 @@ def test_uxstommelgyre_pset_execute():
         pclass=Particle,
     )
     pset.execute(
+        AdvectionEE,
         runtime=np.timedelta64(10, "m"),
         dt=np.timedelta64(60, "s"),
-        pyfunc=AdvectionEE,
     )
     assert utils.round_and_hash_float_array([p.lon for p in pset]) == 1165396086
     assert utils.round_and_hash_float_array([p.lat for p in pset]) == 1142124776