Merge pull request #1779 from OceanParcels/v/1637

Cleanup macOS debugging files

Author: VeckoTheGecko

parcels/__init__.py

@@ -2,7 +2,7 @@
 
 __version__ = version
 
-import parcels.rng as ParcelsRandom  # noqa
+import parcels.rng as ParcelsRandom  # noqa: F401
 from parcels.application_kernels import *
 from parcels.field import *
 from parcels.fieldset import *

parcels/_version.py

@@ -1,5 +1,6 @@
 import os
 import subprocess
+import warnings
 
 try:
     from parcels._version_setup import version
@@ -13,9 +14,11 @@
             .decode("ascii")
             .strip()
         )
-    except subprocess.SubprocessError as e:
-        e.add_note(
+    except subprocess.SubprocessError:
+        warnings.warn(
             "Looks like you're trying to do a development install of parcels. "
             "This needs to be in a git repo so that version information is available. "
+            "Setting version to 'unknown'.",
+            stacklevel=2,
         )
-        raise e
+        version = "unknown"

parcels/kernel.py

@@ -4,9 +4,10 @@
 import functools
 import hashlib
 import inspect
-import math  # noqa
+import math  # noqa: F401
 import os
-import random  # noqa
+import random  # noqa: F401
+import shutil
 import sys
 import textwrap
 import types
@@ -17,10 +18,9 @@
 
 import numpy as np
 import numpy.ctypeslib as npct
-from numpy import ndarray
 
-import parcels.rng as ParcelsRandom  # noqa
-from parcels import rng  # noqa
+import parcels.rng as ParcelsRandom  # noqa: F401
+from parcels import rng  # noqa: F401
 from parcels._compat import MPI
 from parcels.application_kernels.advection import (
     AdvectionAnalytical,
@@ -76,19 +76,14 @@ def __init__(
         self.funcvars = funcvars
         self.funccode = funccode
         self.py_ast = py_ast
-        self.dyn_srcs = []
-        self.src_file = None
-        self.lib_file = None
-        self.log_file = None
+        self.src_file: str | None = None
+        self.lib_file: str | None = None
+        self.log_file: str | None = None
         self.scipy_positionupdate_kernels_added = False
 
         # Generate the kernel function and add the outer loop
         if self._ptype.uses_jit:
-            src_file_or_files, self.lib_file, self.log_file = self.get_kernel_compile_files()
-            if type(src_file_or_files) in (list, dict, tuple, ndarray):
-                self.dyn_srcs = src_file_or_files
-            else:
-                self.src_file = src_file_or_files
+            self.src_file, self.lib_file, self.log_file = self.get_kernel_compile_files()
 
     def __del__(self):
         # Clean-up the in-memory dynamic linked libraries.
@@ -139,7 +134,10 @@ def remove_deleted(self, pset):
         pset.remove_indices(indices)
 
     @abc.abstractmethod
-    def get_kernel_compile_files(self): ...
+    def get_kernel_compile_files(self) -> tuple[str, str, str]: ...
+
+    @abc.abstractmethod
+    def remove_lib(self) -> None: ...
 
 
 class Kernel(BaseKernel):
@@ -272,25 +270,7 @@ def __init__(
                 c_include_str = self._c_include
             self.ccode = loopgen.generate(self.funcname, self.field_args, self.const_args, kernel_ccode, c_include_str)
 
-            src_file_or_files, self.lib_file, self.log_file = self.get_kernel_compile_files()
-            if type(src_file_or_files) in (list, dict, tuple, np.ndarray):
-                self.dyn_srcs = src_file_or_files
-            else:
-                self.src_file = src_file_or_files
-
-    def __del__(self):
-        # Clean-up the in-memory dynamic linked libraries.
-        # This is not really necessary, as these programs are not that large, but with the new random
-        # naming scheme which is required on Windows OS'es to deal with updates to a Parcels' kernel.
-        try:
-            self.remove_lib()
-        except:
-            pass
-        self._fieldset = None
-        self.field_args = None
-        self.const_args = None
-        self.funcvars = None
-        self.funccode = None
+            self.src_file, self.lib_file, self.log_file = self.get_kernel_compile_files()
 
     @property
     def ptype(self):
@@ -330,9 +310,9 @@ def Setcoords(particle, fieldset, time):
             particle.time = particle.time_nextloop
 
         def Updatecoords(particle, fieldset, time):
-            particle.lon_nextloop = particle.lon + particle_dlon  # noqa
-            particle.lat_nextloop = particle.lat + particle_dlat  # noqa
-            particle.depth_nextloop = particle.depth + particle_ddepth  # noqa
+            particle.lon_nextloop = particle.lon + particle_dlon  # type: ignore[name-defined] # noqa
+            particle.lat_nextloop = particle.lat + particle_dlat  # type: ignore[name-defined] # noqa
+            particle.depth_nextloop = particle.depth + particle_ddepth  # type: ignore[name-defined] # noqa
             particle.time_nextloop = particle.time + particle.dt
 
         self._pyfunc = (Setcoords + self + Updatecoords)._pyfunc
@@ -412,29 +392,22 @@ def remove_lib(self):
             del self._lib
             self._lib = None
 
-        all_files_array = []
-        if self.src_file is None:
-            if self.dyn_srcs is not None:
-                [all_files_array.append(fpath) for fpath in self.dyn_srcs]
-        else:
-            if self.src_file is not None:
-                all_files_array.append(self.src_file)
+        all_files: list[str] = []
+        if self.src_file is not None:
+            all_files.append(self.src_file)
         if self.log_file is not None:
-            all_files_array.append(self.log_file)
-        if self.lib_file is not None and all_files_array is not None and self.delete_cfiles is not None:
-            self.cleanup_remove_files(self.lib_file, all_files_array, self.delete_cfiles)
+            all_files.append(self.log_file)
+        if self.lib_file is not None:
+            self.cleanup_remove_files(self.lib_file, all_files, self.delete_cfiles)
 
         # If file already exists, pull new names. This is necessary on a Windows machine, because
         # Python's ctype does not deal in any sort of manner well with dynamic linked libraries on this OS.
         if self._ptype.uses_jit:
-            src_file_or_files, self.lib_file, self.log_file = self.get_kernel_compile_files()
-            if type(src_file_or_files) in (list, dict, tuple, ndarray):
-                self.dyn_srcs = src_file_or_files
-            else:
-                self.src_file = src_file_or_files
+            self.src_file, self.lib_file, self.log_file = self.get_kernel_compile_files()
 
     def get_kernel_compile_files(self):
         """Returns the correct src_file, lib_file, log_file for this kernel."""
+        basename: str
         if MPI:
             mpi_comm = MPI.COMM_WORLD
             mpi_rank = mpi_comm.Get_rank()
@@ -453,39 +426,26 @@ def get_kernel_compile_files(self):
             dyn_dir = get_cache_dir()
             basename = f"{cache_name}_0"
         lib_path = "lib" + basename
-        src_file_or_files = None
-        if type(basename) in (list, dict, tuple, ndarray):
-            src_file_or_files = [""] * len(basename)
-            for i, src_file in enumerate(basename):
-                src_file_or_files[i] = f"{os.path.join(dyn_dir, src_file)}.c"
-        else:
-            src_file_or_files = f"{os.path.join(dyn_dir, basename)}.c"
+
+        assert isinstance(basename, str)
+
+        src_file = f"{os.path.join(dyn_dir, basename)}.c"
         lib_file = f"{os.path.join(dyn_dir, lib_path)}.{'dll' if sys.platform == 'win32' else 'so'}"
         log_file = f"{os.path.join(dyn_dir, basename)}.log"
-        return src_file_or_files, lib_file, log_file
+        return src_file, lib_file, log_file
 
     def compile(self, compiler):
         """Writes kernel code to file and compiles it."""
-        all_files_array = []
         if self.src_file is None:
-            if self.dyn_srcs is not None:
-                for dyn_src in self.dyn_srcs:
-                    with open(dyn_src, "w") as f:
-                        f.write(self.ccode)
-                    all_files_array.append(dyn_src)
-                compiler.compile(self.dyn_srcs, self.lib_file, self.log_file)
-        else:
-            if self.src_file is not None:
-                with open(self.src_file, "w") as f:
-                    f.write(self.ccode)
-                if self.src_file is not None:
-                    all_files_array.append(self.src_file)
-                compiler.compile(self.src_file, self.lib_file, self.log_file)
-        if len(all_files_array) > 0:
-            if self.delete_cfiles is False:
-                logger.info(f"Compiled {self.name} ==> {self.src_file}")
-            if self.log_file is not None:
-                all_files_array.append(self.log_file)
+            return
+
+        with open(self.src_file, "w") as f:
+            f.write(self.ccode)
+
+        compiler.compile(self.src_file, self.lib_file, self.log_file)
+
+        if self.delete_cfiles is False:
+            logger.info(f"Compiled {self.name} ==> {self.src_file}")
 
     def load_lib(self):
         self._lib = npct.load_library(self.lib_file, ".")
@@ -558,14 +518,22 @@ def from_list(cls, fieldset, ptype, pyfunc_list, *args, **kwargs):
         return functools.reduce(lambda x, y: x + y, pyfunc_list)
 
     @staticmethod
-    def cleanup_remove_files(lib_file, all_files_array, delete_cfiles):
-        if lib_file is not None:
-            if os.path.isfile(lib_file):  # and delete_cfiles
-                os.remove(lib_file)
-            if delete_cfiles:
-                for s in all_files_array:
-                    if os.path.exists(s):
-                        os.remove(s)
+    def cleanup_remove_files(lib_file: str | None, all_files: list[str], delete_cfiles: bool) -> None:
+        if lib_file is None:
+            return
+
+        # Remove compiled files
+        if os.path.isfile(lib_file):
+            os.remove(lib_file)
+
+        macos_debugging_files = f"{lib_file}.dSYM"
+        if os.path.isdir(macos_debugging_files):
+            shutil.rmtree(macos_debugging_files)
+
+        if delete_cfiles:
+            for s in all_files:
+                if os.path.exists(s):
+                    os.remove(s)
 
     @staticmethod
     def cleanup_unload_lib(lib):

parcels/tools/global_statics.py

@@ -3,13 +3,16 @@
 import sys
 from pathlib import Path
 from tempfile import gettempdir
+from typing import Literal
 
+USER_ID: int | Literal["tmp"]
 try:
     from os import getuid
+
+    USER_ID = getuid()
 except:
-    # Windows does not have getuid(), so define to simply return 'tmp'
-    def getuid():  # type: ignore
-        return "tmp"
+    # Windows does not have getuid()
+    USER_ID = "tmp"
 
 
 __all__ = ["cleanup_remove_files", "cleanup_unload_lib", "get_package_dir", "get_cache_dir"]
@@ -34,6 +37,6 @@ def get_package_dir():
 
 
 def get_cache_dir():
-    directory = os.path.join(gettempdir(), f"parcels-{getuid()}")
+    directory = os.path.join(gettempdir(), f"parcels-{USER_ID}")
     Path(directory).mkdir(exist_ok=True)
     return directory

tests/test_kernel_execution.py

@@ -1,5 +1,7 @@
 import os
 import sys
+import uuid
+from datetime import timedelta
 
 import numpy as np
 import pytest
@@ -15,11 +17,23 @@
     StatusCode,
 )
 from tests.common_kernels import DeleteParticle, DoNothing, MoveEast, MoveNorth
-from tests.utils import create_fieldset_unit_mesh
+from tests.utils import assert_empty_folder, create_fieldset_unit_mesh, create_fieldset_zeros_simple
 
 ptype = {"scipy": ScipyParticle, "jit": JITParticle}
 
 
+@pytest.fixture()
+def parcels_cache(monkeypatch, tmp_path_factory):
+    """Dedicated folder parcels used to store cached Kernel C code/libraries and log files."""
+    tmp_path = tmp_path_factory.mktemp(f"c-code-{uuid.uuid4()}")
+
+    def fake_get_cache_dir():
+        return tmp_path
+
+    monkeypatch.setattr(parcels.kernel, "get_cache_dir", fake_get_cache_dir)
+    yield tmp_path
+
+
 @pytest.fixture
 def fieldset_unit_mesh():
     return create_fieldset_unit_mesh()
@@ -427,3 +441,16 @@ def outdated_kernel(particle, fieldset, time, dt):
         pset.execute(outdated_kernel, endtime=1.0, dt=1.0)
 
     assert "Since Parcels v2.0" in str(e.value)
+
+
+def test_kernel_file_cleanup(parcels_cache):
+    pset = ParticleSet(create_fieldset_zeros_simple(), pclass=JITParticle, lon=[0.0], lat=[0.0])
+
+    pset.execute(
+        [parcels.AdvectionRK4],
+        runtime=timedelta(minutes=10),
+        dt=timedelta(minutes=5),
+    )
+    del pset  # cleans up compiled C files on deletion
+
+    assert_empty_folder(parcels_cache)

tests/utils.py

@@ -81,3 +81,7 @@ def create_fieldset_zeros_simple(xdim=40, ydim=100):
     data = {"U": np.array(U, dtype=np.float32), "V": np.array(V, dtype=np.float32)}
     dimensions = {"lat": lat, "lon": lon, "depth": depth}
     return FieldSet.from_data(data, dimensions)
+
+
+def assert_empty_folder(path: Path):
+    assert [p.name for p in path.iterdir()] == []