reverted time_step and time_step_output to seconds

docs/whats_new.md

@@ -6,6 +6,8 @@
 * now `do3d=False` and `vertical_mixing=True` can be set at the same time.
 * updated some config descriptions
 * found error in `OilTypeEnum` leading to oil_types with duplicate labels not being separated out in the Enum. This is fixed. Now oil should be input with either the ID from ADIOS or with the syntax `(ID, label)`. Once in the configuration, it will be presented as only `label` since that is how `OpenDrift` accepts `oil_type`.
+* `OilTypeEnum` was reordered so the first 5 oils are most relevant to Alaska work.
+* Reverted `time_step` units back to seconds from minutes. It was being mapped unexpectedly in `OpenDrift` so using seconds is better.
 
 
 ## v0.12.1 (April 8, 2025)

particle_tracking_manager/config_the_manager.py

@@ -122,18 +122,18 @@ class TheManagerConfig(BaseModel):
         True, description="Run forward in time.", json_schema_extra=dict(ptm_level=2)
     )
     time_step: float = Field(
-        5,
+        300,
         ge=0.01,
         le=1440,
-        description="Interval between particles updates, in minutes.",
-        json_schema_extra=dict(ptm_level=3, units="minutes"),
+        description="Interval between particles updates, in seconds.",
+        json_schema_extra=dict(ptm_level=3, units="seconds"),
     )
     time_step_output: float = Field(
-        60,
+        3600,
         ge=1,
-        le=1440,
+        le=86400,
         description="Time step at which element properties are stored and eventually written to file. This must be larger than the calculation time step, and be an integer multiple of this.",
-        json_schema_extra=dict(ptm_level=3, units="minutes"),
+        json_schema_extra=dict(ptm_level=3, units="seconds"),
     )
     steps: int | None = Field(
         None,
@@ -275,7 +275,7 @@ def check_config_time_parameters(self) -> Self:
             assert self.end_time is not None
             duration = pd.Timedelta(abs(self.end_time - self.start_time)).isoformat()
             steps = int(
-                abs(self.end_time - self.start_time) / timedelta(minutes=self.time_step)
+                abs(self.end_time - self.start_time) / timedelta(seconds=self.time_step)
             )
             if duration != self.duration:
                 raise ValueError(
@@ -310,13 +310,13 @@ def calculate_config_times(self) -> Self:
         if self.steps is None:
             if self.duration is not None:
                 self.steps = int(
-                    pd.Timedelta(self.duration) / pd.Timedelta(minutes=self.time_step)
+                    pd.Timedelta(self.duration) / pd.Timedelta(seconds=self.time_step)
                 )
                 logger.debug(f"Setting steps to {self.steps} based on duration.")
             elif self.end_time is not None and self.start_time is not None:
                 self.steps = int(
                     abs(self.end_time - self.start_time)
-                    / timedelta(minutes=self.time_step)
+                    / timedelta(seconds=self.time_step)
                 )
                 logger.debug(
                     f"Setting steps to {self.steps} based on end_time and start_time."
@@ -336,18 +336,18 @@ def calculate_config_times(self) -> Self:
                 )
             elif self.steps is not None:
                 self.duration = pd.Timedelta(
-                    self.steps * timedelta(minutes=self.time_step)
+                    self.steps * timedelta(seconds=self.time_step)
                 ).isoformat()
                 # # convert to ISO 8601 string
-                # self.duration = (self.steps * pd.Timedelta(minutes=self.time_step)).isoformat()
+                # self.duration = (self.steps * pd.Timedelta(seconds=self.time_step)).isoformat()
                 logger.debug(f"Setting duration to {self.duration} based on steps.")
             else:
                 raise ValueError("duration has not been calculated")
 
         if self.end_time is None:
             if self.steps is not None and self.start_time is not None:
                 self.end_time = self.start_time + self.timedir * self.steps * timedelta(
-                    minutes=self.time_step
+                    seconds=self.time_step
                 )
                 logger.debug(
                     f"Setting end_time to {self.end_time} based on start_time and steps."
@@ -363,7 +363,7 @@ def calculate_config_times(self) -> Self:
         if self.start_time is None:
             if self.end_time is not None and self.steps is not None:
                 self.start_time = self.end_time - self.timedir * self.steps * timedelta(
-                    minutes=self.time_step
+                    seconds=self.time_step
                 )
                 logger.debug(
                     f"Setting start_time to {self.start_time} based on end_time and steps."

particle_tracking_manager/models/opendrift/config_opendrift.py

@@ -151,14 +151,16 @@ class OpenDriftConfig(TheManagerConfig):
         TheManagerConfig.model_fields["number"],
         Field(json_schema_extra=dict(od_mapping="seed:number")),
     )
-    time_step: float = FieldInfo.merge_field_infos(
-        TheManagerConfig.model_fields["time_step"],
-        Field(json_schema_extra=dict(od_mapping="general:time_step_minutes")),
-    )
-    time_step_output: float = FieldInfo.merge_field_infos(
-        TheManagerConfig.model_fields["time_step_output"],
-        Field(json_schema_extra=dict(od_mapping="general:time_step_output_minutes")),
-    )
+    # These don't properly map the way I expect in OpenDrift. It is better to leave time_step as not
+    # associated with an `od_mapping` to avoid confusion.
+    # time_step: float = FieldInfo.merge_field_infos(
+    #     TheManagerConfig.model_fields["time_step"],
+    #     Field(json_schema_extra=dict(od_mapping="general:time_step_minutes")),
+    # )
+    # time_step_output: float = FieldInfo.merge_field_infos(
+    #     TheManagerConfig.model_fields["time_step_output"],
+    #     Field(json_schema_extra=dict(od_mapping="general:time_step_output_minutes")),
+    # )
 
     model_config = {
         "validate_defaults": True,

tests/test_config_the_manager.py

@@ -38,14 +38,14 @@ def test_time_calculations():
 
     # all times defined but consistently
     # duration is 1 time step
-    ts, duration = 5, "P0DT0H5M0S"
+    ts, duration = 5 * 60, "P0DT0H5M0S"
     start_time = "2022-01-01 12:00:00"
     m = TheManagerConfig(
         time_step=ts,
         steps=1,
         duration=duration,
         start_time=start_time,
-        end_time=pd.Timestamp(start_time) + pd.Timedelta(minutes=ts),
+        end_time=pd.Timestamp(start_time) + pd.Timedelta(seconds=ts),
     )
 
     m = TheManagerConfig(steps=1, end_time="2022-01-01 12:00:00", start_time=None)
@@ -177,13 +177,13 @@ def test_misc_parameters():
         start_time="2022-01-01",
         horizontal_diffusivity=1,
         number=100,
-        time_step=5,
+        time_step=50,
         stokes_drift=False,
     )
 
     assert m.horizontal_diffusivity == 1
     assert m.number == 100
-    assert m.time_step == 5
+    assert m.time_step == 50
     assert m.stokes_drift == False
 
 

tests/test_opendrift.py

@@ -392,7 +392,7 @@ def test_plots_names():
 def test_parameter_passing():
     """make sure parameters passed into package make it to simulation runtime."""
 
-    ts = 7  # minutes
+    ts = 7 * 60  # seconds
     diffmodel = "windspeed_Sundby1983"
     use_auto_landmask = True
     vertical_mixing = True
@@ -408,7 +408,7 @@ def test_parameter_passing():
     m = OpenDriftModel(
         use_auto_landmask=use_auto_landmask,
         time_step=ts,
-        duration="P0DT10H0M0S",
+        duration="P0DT0H5M0S",
         steps=None,
         diffusivitymodel=diffmodel,
         vertical_mixing=vertical_mixing,
@@ -423,10 +423,11 @@ def test_parameter_passing():
 
     # check time_step across access points
     assert (
-        m.o._config["general:time_step_minutes"]["value"]
-        == ts
+        # m.o._config["general:time_step_minutes"]["value"]  # this is not correct, don't know why
+        # m.o.time_step.total_seconds()  # this is only created once run
+        ts
         == m.config.time_step
-        == m.o.get_configspec()["general:time_step_minutes"]["value"]
+        # == m.o.get_configspec()["general:time_step_minutes"]["value"]  # this is not correct, don't know why
     )
 
     # check diff model

tests/test_realistic.py

@@ -66,7 +66,7 @@ def test_run_parquet():
         steps=2,
         output_format="parquet",
         ocean_model="TXLA",
-        ocean_model_local=False
+        ocean_model_local=False,
     )
     manager.run_all()
 
@@ -79,6 +79,8 @@ def test_run_netcdf_and_plot():
 
     import tempfile
 
+    ts = 6 * 60  # 6 minutes in seconds
+
     seeding_kwargs = dict(lon=-90, lat=28.7, number=1, start_time="2009-11-19T12:00:00")
     with tempfile.NamedTemporaryFile(delete=False) as temp_file:
         manager = ptm.OpenDriftModel(
@@ -92,7 +94,8 @@ def test_run_netcdf_and_plot():
             ocean_model_local=False,
             plots={
                 "all": {},
-            }
+            },
+            time_step=ts,
         )
         manager.run_all()
 
@@ -106,3 +109,12 @@ def test_run_netcdf_and_plot():
             data = pickle.load(file)
         assert "spl_x" in data
         assert "spl_y" in data
+
+    # check time_step across access points
+    assert (
+        # m.o._config["general:time_step_minutes"]["value"]  # this is not correct, don't know why
+        manager.o.time_step.total_seconds()
+        == ts
+        == manager.config.time_step
+        # == m.o.get_configspec()["general:time_step_minutes"]["value"]  # this is not correct, don't know why
+    )