Merge pull request #25 from axiom-data-science/updates

removed Dcrit, other updates

docs/whats_new.md

@@ -1,5 +1,11 @@
 # What's New
 
+## v0.8.3 (April 23, 2024)
+
+* removed `Dcrit` because realized it is not necessary
+* improved log handling for CLI
+* changed `OpenDrift` default handling so they are now changed to None
+
 ## v0.8.2 (April 10, 2024)
 
 * updated docs

particle_tracking_manager/cli.py

@@ -2,6 +2,7 @@
 
 import argparse
 import ast
+import logging
 
 from datetime import datetime
 
@@ -122,6 +123,23 @@ def main():
     }
     args.kwargs.update(to_bool)
 
+    if "output_file" not in args.kwargs:
+
+        args.kwargs[
+            "output_file"
+        ] = f"output-results_{datetime.utcnow():%Y-%m-%dT%H%M:%SZ}.nc"
+
+    log_file = args.kwargs["output_file"].replace(".nc", ".log")
+
+    # Create a file handler
+    file_handler = logging.FileHandler(log_file)
+
+    # Create a formatter and add it to the handler
+    formatter = logging.Formatter(
+        "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
+    )
+    file_handler.setFormatter(formatter)
+
     m = ptm.OpenDriftModel(**args.kwargs)
 
     if args.dry_run:
@@ -132,10 +150,19 @@ def main():
 
     else:
 
+        # Add the handler to the logger
+        m.logger.addHandler(file_handler)
+
+        m.logger.info(f"filename: {args.kwargs['output_file']}")
+
         m.add_reader()
         print(m.drift_model_config())
 
         m.seed()
         m.run()
 
         print(m.outfile_name)
+
+    # Remove the handler at the end of the loop
+    m.logger.removeHandler(file_handler)
+    file_handler.close()

particle_tracking_manager/models/opendrift/config.json

@@ -65,7 +65,7 @@
         "ptm_level": 1
     },
     "max_speed": {
-        "default": 2,
+        "default": 5,
         "od_mapping": "drift:max_speed"
     },
     "horizontal_diffusivity": {
@@ -204,10 +204,5 @@
         "default": "low",
         "ptm_level": 2,
         "description": "Log verbosity"
-    },
-    "Dcrit": {
-        "default": 0.1,
-        "od_mapping": "general:seafloor_action_dcrit",
-        "ptm_level": 3
     }
 }

particle_tracking_manager/models/opendrift/opendrift.py

@@ -327,15 +327,6 @@ def __setattr_model__(self, name: str, value) -> None:
             self.config_model[name]["value"] = value
         self._update_config()
 
-        if name == "ocean_model":
-            if value == "NWGOA":
-                self.Dcrit = 0.5
-            elif "CIOFS" in value:
-                self.Dcrit = 0.3
-            else:
-                self.Dcrit = 0.1
-            self.logger.info(f"For ocean_model {value}, setting Dcrit to {self.Dcrit}.")
-
         if name in ["ocean_model", "horizontal_diffusivity"]:
 
             # just set the value and move on if purposely setting a non-None value
@@ -431,33 +422,24 @@ def __setattr_model__(self, name: str, value) -> None:
                 "drift_model is LarvalFish which is always 3D in OpenDrift so do3D must be True."
             )
 
-        # Make sure vertical_mixing_timestep equals default value if vertical_mixing False
+        # Make sure vertical_mixing_timestep equals None if vertical_mixing False
         if name in ["vertical_mixing", "vertical_mixing_timestep"]:
-            vmtdef = self.config_model["vertical_mixing_timestep"]["default"]
-            if (
-                not self.vertical_mixing
-                and self.vertical_mixing_timestep != vmtdef
-                and self.vertical_mixing_timestep is not None
-            ):
+            if not self.vertical_mixing:
                 self.logger.info(
-                    "vertical_mixing is False, so resetting value of vertical_mixing_timestep to default and not using."
+                    "vertical_mixing is False, so setting value of vertical_mixing_timestep "
+                    "to None."
                 )
-                self.__dict__["vertical_mixing_timestep"] = vmtdef
-                self.config_model["vertical_mixing_timestep"]["value"] = vmtdef
+                self.__dict__["vertical_mixing_timestep"] = None
+                self.config_model["vertical_mixing_timestep"]["value"] = None
 
-        # Make sure diffusivitymodel equals default value if vertical_mixing False
+        # Make sure diffusivitymodel equals None if vertical_mixing False
         if name in ["vertical_mixing", "diffusivitymodel"]:
-            dmodeldef = self.config_model["diffusivitymodel"]["default"]
-            if (
-                not self.vertical_mixing
-                and self.diffusivitymodel != dmodeldef
-                and self.diffusivitymodel is not None
-            ):
+            if not self.vertical_mixing:
                 self.logger.info(
-                    "vertical_mixing is False, so resetting value of diffusivitymodel to default and not using."
+                    "vertical_mixing is False, so setting value of diffusivitymodel to None."
                 )
-                self.__dict__["diffusivitymodel"] = dmodeldef
-                self.config_model["diffusivitymodel"]["value"] = dmodeldef
+                self.__dict__["diffusivitymodel"] = None
+                self.config_model["diffusivitymodel"]["value"] = None
 
         # Make sure mixed_layer_depth equals default value if vertical_mixing False
         if name in ["vertical_mixing", "mixed_layer_depth"]:
@@ -473,36 +455,28 @@ def __setattr_model__(self, name: str, value) -> None:
                 self.__dict__["mixed_layer_depth"] = mlddef
                 self.config_model["mixed_layer_depth"]["value"] = mlddef
 
-        # make sure user isn't try to use Leeway and "wind_drift_factor" at the same time
-        if name in ["drift_model", "wind_drift_factor"]:
-            mdfdef = self.config_model["wind_drift_factor"]["default"]
-            if (
-                self.drift_model == "Leeway"
-                and self.wind_drift_factor != mdfdef
-                and self.wind_drift_factor is not None
-            ):
+        # make sure user isn't try to use Leeway or LarvalFish and "wind_drift_factor" at the same time
+        if name == "wind_drift_factor":
+            if self.drift_model in ["Leeway", "LarvalFish"]:
                 self.logger.info(
-                    "wind_drift_factor cannot be used with Leeway model, so resetting value to default and not using."
+                    "wind_drift_factor cannot be used with Leeway or LarvalFish models, "
+                    "so setting to None."
                 )
-                self.__dict__["wind_drift_factor"] = mdfdef
-                self.config_model["wind_drift_factor"]["value"] = mdfdef
+                self.__dict__["wind_drift_factor"] = None
+                self.config_model["wind_drift_factor"]["value"] = None
 
-        # make sure user isn't try to use Leeway and "wind_drift_depth" at the same time
-        if name in ["drift_model", "wind_drift_depth"]:
-            mdddef = self.config_model["wind_drift_depth"]["default"]
-            if (
-                self.drift_model == "Leeway"
-                and self.wind_drift_depth != mdddef
-                and self.wind_drift_depth is not None
-            ):
+        # make sure user isn't try to use Leeway or LarvalFish models and "wind_drift_depth" at the same time
+        if name == "wind_drift_depth":
+            if self.drift_model in ["Leeway", "LarvalFish"]:
                 self.logger.info(
-                    "wind_drift_depth cannot be used with Leeway model, so resetting value to default and not using."
+                    "wind_drift_depth cannot be used with Leeway or LarvalFish models, "
+                    "so setting to None."
                 )
-                self.__dict__["wind_drift_depth"] = mdddef
-                self.config_model["wind_drift_depth"]["value"] = mdddef
+                self.__dict__["wind_drift_depth"] = None
+                self.config_model["wind_drift_depth"]["value"] = None
 
         # make sure user isn't try to use Leeway and "stokes_drift" at the same time
-        if name in ["drift_model", "stokes_drift"]:
+        if name == "stokes_drift":
             if self.drift_model == "Leeway" and self.stokes_drift:
                 self.logger.info(
                     "stokes_drift cannot be used with Leeway model, so changing to False."
@@ -1088,7 +1062,7 @@ def drift_model_config(self, ptm_level=[1, 2, 3], prefix=""):
             for key, value_dict in self.show_config(
                 substring=":", ptm_level=ptm_level, level=[1, 2, 3], prefix=prefix
             ).items()
-            if "value" in value_dict
+            if "value" in value_dict and value_dict["value"] is not None
         ]
 
         # also PTM config parameters that are separate from OpenDrift parameters
@@ -1097,7 +1071,9 @@ def drift_model_config(self, ptm_level=[1, 2, 3], prefix=""):
             for key, value_dict in self.show_config(
                 ptm_level=ptm_level, prefix=prefix
             ).items()
-            if "od_mapping" not in value_dict and "value" in value_dict
+            if "od_mapping" not in value_dict
+            and "value" in value_dict
+            and value_dict["value"] is not None
         ]
 
         # extra parameters that are not in the config_model but are set by PTM indirectly

tests/test_opendrift.py

@@ -25,9 +25,7 @@ def test_init(self):
         self.assertEqual(
             self.odm.use_auto_landmask, config_model["use_auto_landmask"]["default"]
         )
-        self.assertEqual(
-            self.odm.diffusivitymodel, config_model["diffusivitymodel"]["default"]
-        )
+        self.assertEqual(self.odm.diffusivitymodel, None)
         self.assertEqual(self.odm.stokes_drift, config_model["stokes_drift"]["default"])
         self.assertEqual(
             self.odm.mixed_layer_depth, config_model["mixed_layer_depth"]["default"]
@@ -44,7 +42,7 @@ def test_init(self):
         )
         self.assertEqual(
             self.odm.vertical_mixing_timestep,
-            config_model["vertical_mixing_timestep"]["default"],
+            None,
         )
         self.assertEqual(self.odm.object_type, config_model["object_type"]["default"])
         self.assertEqual(self.odm.diameter, config_model["diameter"]["default"])
@@ -275,19 +273,19 @@ def test_vertical_mixing_false_vertical_mixing_timestep_not_default(self):
         self.m.vertical_mixing = False
         self.m.vertical_mixing_timestep = 10
         d = self.m.show_config(key="vertical_mixing_timestep")
-        assert d["value"] == d["default"]
+        assert d["value"] == None
 
     def test_vertical_mixing_false_diffusivitymodel_not_default(self):
         self.m.vertical_mixing = False
         self.m.diffusivitymodel = "not_default"
         d = self.m.show_config(key="diffusivitymodel")
-        assert d["value"] == d["default"]
+        assert d["value"] == None
 
     def test_vertical_mixing_false_mixed_layer_depth_not_default(self):
         self.m.vertical_mixing = False
         self.m.mixed_layer_depth = 10
         d = self.m.show_config(key="mixed_layer_depth")
-        assert d["value"] == d["default"]
+        assert d["value"] == 30
 
 
 class TestOpenDriftModel_Leeway(unittest.TestCase):
@@ -299,14 +297,14 @@ def setUp(self):
     def test_leeway_model_wind_drift_factor_not_default(self):
         self.m.wind_drift_factor = 10
         d = self.m.show_config(key="wind_drift_factor")
-        assert d["value"] == d["default"]
+        assert d["value"] == None
         assert self.m.object_type == ">PIW, scuba suit (face up)"
         assert self.m.o._config["object_type"]["value"] == ">PIW, scuba suit (face up)"
 
     def test_leeway_model_wind_drift_depth_not_default(self):
         self.m.wind_drift_depth = 10
         d = self.m.show_config(key="wind_drift_depth")
-        assert d["value"] == d["default"]
+        assert d["value"] == None
 
     def test_leeway_model_stokes_drift_true(self):
         self.m.stokes_drift = True