From 23cd3fe2d082419d10398cb1a596484bf0f6e8d7 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:01:13 +0200 Subject: [PATCH 1/8] add tmp file write/read step --- src/virtualship/instruments/base.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index cc406def..f5d285e9 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -2,6 +2,7 @@ import abc import collections +import tempfile from datetime import timedelta from itertools import pairwise from pathlib import Path @@ -183,6 +184,9 @@ def _generate_fieldset(self) -> parcels.FieldSet: Create and combine FieldSets for each variable, supporting both local and Copernicus Marine data sources. N.B. Per variable avoids issues when using copernicusmarine and creating directly one FieldSet of ds's sourced from different Copernicus Marine product IDs (which can also have different temporal resolutions), which is often the case for BGC variables. + + Includes an intermediate step of writing to tmp files, as per https://github.com/Parcels-code/parcels-benchmarks/pull/49 + TODO: the need for this step may be removed as Parcels x copernicusmarine integration improves, tracked in https://github.com/Parcels-code/Parcels/issues/2756 and xref'd in VirtualShip #357 (https://github.com/Parcels-code/virtualship/issues/357) """ fieldsets_list = [] keys = list(self.variables.keys()) @@ -222,11 +226,12 @@ def _generate_fieldset(self) -> parcels.FieldSet: ds["depth"] = -ds["depth"] ds = ds.reindex(depth=ds["depth"][::-1]) - # TODO: update when decision on handling of nans/0s in v4 is made (i.e. https://github.com/Parcels-code/Parcels/issues/2393) + # TODO: to be removed when Parcels #2746 is merged (i.e. https://github.com/Parcels-code/Parcels/pull/2746) ds = ds.fillna(0) fields = {key: ds[field_var_name]} ds_fset = parcels.convert.copernicusmarine_to_sgrid(fields=fields) + ds_fset = self._via_tmp_ds(ds_fset) fs = parcels.FieldSet.from_sgrid_conventions(ds_fset) @@ -253,3 +258,12 @@ def _get_spec_value(self, spec_type: str, key: str, default=None): """Helper to extract a value from spacetime_buffer_size or limit_spec.""" spec = self.spacetime_buffer_size if spec_type == "buffer" else self.limit_spec return spec.get(key) if spec and spec.get(key) is not None else default + + @staticmethod + def _via_tmp_ds(ds) -> xr.Dataset: + """Create and re-load a temporary local dataset.""" + tmpdir = tempfile.TemporaryDirectory() + tmp_fpath = Path(tmpdir.name).joinpath("tmp.nc") + ds.to_netcdf(tmp_fpath) + del ds + return xr.open_dataset(tmp_fpath) From f60d76909bb7a7329207c1a423080f5ad8832ff0 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:14:31 +0200 Subject: [PATCH 2/8] small refactor: consolidate spacetime_buffer_size and limit_spec dicts for clarity --- src/virtualship/instruments/adcp.py | 9 ++++---- src/virtualship/instruments/argo_float.py | 9 +++----- src/virtualship/instruments/base.py | 23 ++++++------------- src/virtualship/instruments/ctd.py | 9 ++++---- src/virtualship/instruments/drifter.py | 15 ++++++------ .../instruments/ship_underwater_st.py | 9 +++----- src/virtualship/instruments/xbt.py | 9 ++++---- 7 files changed, 32 insertions(+), 51 deletions(-) diff --git a/src/virtualship/instruments/adcp.py b/src/virtualship/instruments/adcp.py index 49a08120..492b5a8f 100644 --- a/src/virtualship/instruments/adcp.py +++ b/src/virtualship/instruments/adcp.py @@ -61,9 +61,9 @@ class ADCPInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize ADCPInstrument.""" variables = expedition.instruments_config.adcp_config.active_variables() - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer + fetch_spec = { + "spatial": True, + } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -71,8 +71,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=None, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/argo_float.py b/src/virtualship/instruments/argo_float.py index b18c91ba..0d88c1db 100644 --- a/src/virtualship/instruments/argo_float.py +++ b/src/virtualship/instruments/argo_float.py @@ -253,14 +253,12 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for argo float simulation; sensor variables come from config - spacetime_buffer_size = { + fetch_spec = { + "spatial": True, "latlon": 3.0, # [degrees] "time": expedition.instruments_config.argo_float_config.lifetime.total_seconds() / (24 * 3600), # [days] } - limit_spec = { - "spatial": True, # spatial limits; lat/lon constrained to waypoint locations + buffer - } super().__init__( expedition, @@ -268,8 +266,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=False, verbose_progress=True, - spacetime_buffer_size=spacetime_buffer_size, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index 1595f5e4..8012cd35 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -50,8 +50,7 @@ def __init__( allow_time_extrapolation: bool, verbose_progress: bool, from_data: Path | None, - spacetime_buffer_size: dict | None = None, - limit_spec: dict | None = None, + fetch_spec: dict | None = None, ): """Initialise instrument.""" self.expedition = expedition @@ -67,8 +66,7 @@ def __init__( self.add_bathymetry = add_bathymetry self.allow_time_extrapolation = allow_time_extrapolation self.verbose_progress = verbose_progress - self.spacetime_buffer_size = spacetime_buffer_size - self.limit_spec = limit_spec + self.fetch_spec = fetch_spec wp_lats, wp_lons = _get_waypoint_latlons(expedition.schedule.waypoints) wp_times = [ @@ -153,11 +151,11 @@ def _get_copernicus_ds( ) latlon_buffer = self._get_spec_value( - "buffer", "latlon", 0.25 + "latlon", 0.25 ) # [degrees]; default 0.25 deg buffer to ensure coverage in field cell edge cases - depth_min = self._get_spec_value("limit", "depth_min", None) - depth_max = self._get_spec_value("limit", "depth_max", None) - spatial_constraint = self._get_spec_value("limit", "spatial", True) + depth_min = self._get_spec_value("depth_min", None) + depth_max = self._get_spec_value("depth_max", None) + spatial_constraint = self._get_spec_value("spatial", True) min_lon_bound = self.min_lon - latlon_buffer if spatial_constraint else None max_lon_bound = self.max_lon + latlon_buffer if spatial_constraint else None @@ -176,8 +174,6 @@ def _get_copernicus_ds( minimum_depth=depth_min, maximum_depth=depth_max, coordinates_selection_method="outside", - service="arco-geo-series", - chunk_size_limit=1, vertical_axis="elevation", ) @@ -190,7 +186,7 @@ def _generate_fieldset(self) -> parcels.FieldSet: fieldsets_list = [] keys = list(self.variables.keys()) - time_buffer = self._get_spec_value("buffer", "time", 0.0) + time_buffer = self._get_spec_value("time", 0.0) for key in keys: var = self.variables[key] @@ -251,8 +247,3 @@ def _generate_fieldset(self) -> parcels.FieldSet: base_fieldset.add_field(uv) return base_fieldset - - def _get_spec_value(self, spec_type: str, key: str, default=None): - """Helper to extract a value from spacetime_buffer_size or limit_spec.""" - spec = self.spacetime_buffer_size if spec_type == "buffer" else self.limit_spec - return spec.get(key) if spec and spec.get(key) is not None else default diff --git a/src/virtualship/instruments/ctd.py b/src/virtualship/instruments/ctd.py index 48164ca7..6f0eca3f 100644 --- a/src/virtualship/instruments/ctd.py +++ b/src/virtualship/instruments/ctd.py @@ -142,9 +142,9 @@ class CTDInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize CTDInstrument.""" variables = expedition.instruments_config.ctd_config.active_variables() - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer + fetch_spec = { + "spatial": True, + } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -152,8 +152,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=None, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/drifter.py b/src/virtualship/instruments/drifter.py index a771b246..f2d2351c 100644 --- a/src/virtualship/instruments/drifter.py +++ b/src/virtualship/instruments/drifter.py @@ -88,13 +88,13 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for drifter simulation; sensor variables come from config - spacetime_buffer_size = { - "latlon": None, + fetch_spec = { + "spatial": True, + "latlon": 30.0, # [degrees]; TODO: generous buffer to limit tmp file size download, can potentially be removed in the future as and when Parcels streaming performance improves (see #358) "time": expedition.instruments_config.drifter_config.lifetime.total_seconds() - / (24 * 3600), # [days] - } - limit_spec = { - "spatial": False, # no spatial limits; generate global fieldset + / ( + 24 * 3600 + ), # [days]; TODO: as above, can potentially be removed in the future "depth_min": abs( expedition.instruments_config.drifter_config.depth_meter ), # [meters] @@ -109,8 +109,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=False, verbose_progress=True, - spacetime_buffer_size=spacetime_buffer_size, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/ship_underwater_st.py b/src/virtualship/instruments/ship_underwater_st.py index 302d991f..1563668a 100644 --- a/src/virtualship/instruments/ship_underwater_st.py +++ b/src/virtualship/instruments/ship_underwater_st.py @@ -67,13 +67,11 @@ def __init__(self, expedition, from_data): variables = ( expedition.instruments_config.ship_underwater_st_config.active_variables() ) - spacetime_buffer_size = { + fetch_spec = { + "spatial": True, "latlon": 0.25, # [degrees] "time": 0.0, # [days] } - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -81,8 +79,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=spacetime_buffer_size, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/xbt.py b/src/virtualship/instruments/xbt.py index 92f6b3f2..9b0d6432 100644 --- a/src/virtualship/instruments/xbt.py +++ b/src/virtualship/instruments/xbt.py @@ -95,9 +95,9 @@ class XBTInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize XBTInstrument.""" variables = expedition.instruments_config.xbt_config.active_variables() - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer + fetch_spec = { + "spatial": True, + } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -105,8 +105,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=None, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) From e0187b13db2be3ef5e4e40168350a65fc5090650 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:28:38 +0200 Subject: [PATCH 3/8] to FetchSpec dataclass --- src/virtualship/instruments/adcp.py | 7 ++--- src/virtualship/instruments/argo_float.py | 11 ++++---- src/virtualship/instruments/base.py | 28 +++++++++++++------ src/virtualship/instruments/ctd.py | 7 ++--- src/virtualship/instruments/drifter.py | 19 ++++++------- .../instruments/ship_underwater_st.py | 9 ++---- src/virtualship/instruments/xbt.py | 7 ++--- 7 files changed, 40 insertions(+), 48 deletions(-) diff --git a/src/virtualship/instruments/adcp.py b/src/virtualship/instruments/adcp.py index 492b5a8f..26c8122d 100644 --- a/src/virtualship/instruments/adcp.py +++ b/src/virtualship/instruments/adcp.py @@ -5,7 +5,7 @@ import numpy as np from parcels import ParticleFile, ParticleSet -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.utils import build_particle_class_from_sensors, register_instrument @@ -61,9 +61,6 @@ class ADCPInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize ADCPInstrument.""" variables = expedition.instruments_config.adcp_config.active_variables() - fetch_spec = { - "spatial": True, - } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -71,7 +68,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) diff --git a/src/virtualship/instruments/argo_float.py b/src/virtualship/instruments/argo_float.py index 0d88c1db..96b8e4f0 100644 --- a/src/virtualship/instruments/argo_float.py +++ b/src/virtualship/instruments/argo_float.py @@ -7,7 +7,7 @@ from parcels import ParticleFile, ParticleSet, StatusCode, Variable from parcels.kernels import AdvectionRK2 -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.spacetime import Spacetime @@ -253,12 +253,11 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for argo float simulation; sensor variables come from config - fetch_spec = { - "spatial": True, - "latlon": 3.0, # [degrees] - "time": expedition.instruments_config.argo_float_config.lifetime.total_seconds() + fetch_spec = FetchSpec( + latlon_buffer=3.0, # [degrees] + time_buffer=expedition.instruments_config.argo_float_config.lifetime.total_seconds() / (24 * 3600), # [days] - } + ) super().__init__( expedition, diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index 8012cd35..f07bc0cf 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -2,6 +2,7 @@ import abc import collections +from dataclasses import dataclass from datetime import timedelta from itertools import pairwise from pathlib import Path @@ -28,6 +29,17 @@ from virtualship.models import Expedition +@dataclass +class FetchSpec: + """Fetch constraints and parameters for dataset retrieval.""" + + spatial: bool = True + latlon_buffer: float = 0.25 # degrees + time_buffer: float = 0.0 # days + depth_min: float | None = None + depth_max: float | None = None + + class Instrument(abc.ABC): """Base class for instruments and their simulation.""" @@ -50,7 +62,7 @@ def __init__( allow_time_extrapolation: bool, verbose_progress: bool, from_data: Path | None, - fetch_spec: dict | None = None, + fetch_spec: FetchSpec | None = None, ): """Initialise instrument.""" self.expedition = expedition @@ -66,7 +78,7 @@ def __init__( self.add_bathymetry = add_bathymetry self.allow_time_extrapolation = allow_time_extrapolation self.verbose_progress = verbose_progress - self.fetch_spec = fetch_spec + self.fetch_spec = fetch_spec or FetchSpec() wp_lats, wp_lons = _get_waypoint_latlons(expedition.schedule.waypoints) wp_times = [ @@ -150,12 +162,10 @@ def _get_copernicus_ds( variable=var if not physical else None, ) - latlon_buffer = self._get_spec_value( - "latlon", 0.25 - ) # [degrees]; default 0.25 deg buffer to ensure coverage in field cell edge cases - depth_min = self._get_spec_value("depth_min", None) - depth_max = self._get_spec_value("depth_max", None) - spatial_constraint = self._get_spec_value("spatial", True) + latlon_buffer = self.fetch_spec.latlon_buffer + depth_min = self.fetch_spec.depth_min + depth_max = self.fetch_spec.depth_max + spatial_constraint = self.fetch_spec.spatial min_lon_bound = self.min_lon - latlon_buffer if spatial_constraint else None max_lon_bound = self.max_lon + latlon_buffer if spatial_constraint else None @@ -186,7 +196,7 @@ def _generate_fieldset(self) -> parcels.FieldSet: fieldsets_list = [] keys = list(self.variables.keys()) - time_buffer = self._get_spec_value("time", 0.0) + time_buffer = self.fetch_spec.time_buffer for key in keys: var = self.variables[key] diff --git a/src/virtualship/instruments/ctd.py b/src/virtualship/instruments/ctd.py index 6f0eca3f..d6764130 100644 --- a/src/virtualship/instruments/ctd.py +++ b/src/virtualship/instruments/ctd.py @@ -7,7 +7,7 @@ from parcels import ParticleFile, ParticleSet, Variable from parcels._core.statuscodes import StatusCode -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.utils import ( @@ -142,9 +142,6 @@ class CTDInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize CTDInstrument.""" variables = expedition.instruments_config.ctd_config.active_variables() - fetch_spec = { - "spatial": True, - } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -152,7 +149,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) diff --git a/src/virtualship/instruments/drifter.py b/src/virtualship/instruments/drifter.py index f2d2351c..6319831e 100644 --- a/src/virtualship/instruments/drifter.py +++ b/src/virtualship/instruments/drifter.py @@ -8,7 +8,7 @@ from parcels._core.statuscodes import StatusCode from parcels.kernels import AdvectionRK2 -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.spacetime import Spacetime @@ -88,20 +88,17 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for drifter simulation; sensor variables come from config - fetch_spec = { - "spatial": True, - "latlon": 30.0, # [degrees]; TODO: generous buffer to limit tmp file size download, can potentially be removed in the future as and when Parcels streaming performance improves (see #358) - "time": expedition.instruments_config.drifter_config.lifetime.total_seconds() - / ( - 24 * 3600 - ), # [days]; TODO: as above, can potentially be removed in the future - "depth_min": abs( + fetch_spec = FetchSpec( + latlon_buffer=30.0, # TODO: generous buffer to limit tmp file size download, can potentially be removed in the future as and when Parcels streaming performance improves (see #358) + time_buffer=expedition.instruments_config.drifter_config.lifetime.total_seconds() + / (24 * 3600), # [days] + depth_min=abs( expedition.instruments_config.drifter_config.depth_meter ), # [meters] - "depth_max": abs( + depth_max=abs( expedition.instruments_config.drifter_config.depth_meter ), # [meters] - } + ) super().__init__( expedition, diff --git a/src/virtualship/instruments/ship_underwater_st.py b/src/virtualship/instruments/ship_underwater_st.py index 1563668a..1dc7522a 100644 --- a/src/virtualship/instruments/ship_underwater_st.py +++ b/src/virtualship/instruments/ship_underwater_st.py @@ -5,7 +5,7 @@ import numpy as np from parcels import ParticleFile, ParticleSet -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.utils import ( @@ -67,11 +67,6 @@ def __init__(self, expedition, from_data): variables = ( expedition.instruments_config.ship_underwater_st_config.active_variables() ) - fetch_spec = { - "spatial": True, - "latlon": 0.25, # [degrees] - "time": 0.0, # [days] - } super().__init__( expedition, @@ -79,7 +74,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) diff --git a/src/virtualship/instruments/xbt.py b/src/virtualship/instruments/xbt.py index 9b0d6432..7046fde1 100644 --- a/src/virtualship/instruments/xbt.py +++ b/src/virtualship/instruments/xbt.py @@ -7,7 +7,7 @@ from parcels import ParticleFile, ParticleSet, Variable from parcels._core.statuscodes import StatusCode -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.spacetime import Spacetime @@ -95,9 +95,6 @@ class XBTInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize XBTInstrument.""" variables = expedition.instruments_config.xbt_config.active_variables() - fetch_spec = { - "spatial": True, - } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -105,7 +102,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) From bbb24deed3c2aef3bd0c8a0d73ee71a5f7529a9f Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:01:13 +0200 Subject: [PATCH 4/8] add tmp file write/read step --- src/virtualship/instruments/base.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index f07bc0cf..f281b899 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -2,6 +2,7 @@ import abc import collections +import tempfile from dataclasses import dataclass from datetime import timedelta from itertools import pairwise @@ -192,6 +193,9 @@ def _generate_fieldset(self) -> parcels.FieldSet: Create and combine FieldSets for each variable, supporting both local and Copernicus Marine data sources. N.B. Per variable avoids issues when using copernicusmarine and creating directly one FieldSet of ds's sourced from different Copernicus Marine product IDs (which can also have different temporal resolutions), which is often the case for BGC variables. + + Includes an intermediate step of writing to tmp files, as per https://github.com/Parcels-code/parcels-benchmarks/pull/49 + TODO: the need for this step may be removed as Parcels x copernicusmarine integration improves, tracked in https://github.com/Parcels-code/Parcels/issues/2756 and xref'd in VirtualShip #357 (https://github.com/Parcels-code/virtualship/issues/357) """ fieldsets_list = [] keys = list(self.variables.keys()) @@ -231,11 +235,12 @@ def _generate_fieldset(self) -> parcels.FieldSet: # ds["depth"] = -ds["depth"] # ds = ds.reindex(depth=ds["depth"][::-1]) - # TODO: update when decision on handling of nans/0s in v4 is made (i.e. https://github.com/Parcels-code/Parcels/issues/2393) + # TODO: to be removed when Parcels #2746 is merged (i.e. https://github.com/Parcels-code/Parcels/pull/2746) ds = ds.fillna(0) fields = {key: ds[field_var_name]} ds_fset = parcels.convert.copernicusmarine_to_sgrid(fields=fields) + ds_fset = self._via_tmp_ds(ds_fset) fs = parcels.FieldSet.from_sgrid_conventions(ds_fset) @@ -257,3 +262,12 @@ def _generate_fieldset(self) -> parcels.FieldSet: base_fieldset.add_field(uv) return base_fieldset + + @staticmethod + def _via_tmp_ds(ds) -> xr.Dataset: + """Create and re-load a temporary local dataset.""" + tmpdir = tempfile.TemporaryDirectory() + tmp_fpath = Path(tmpdir.name).joinpath("tmp.nc") + ds.to_netcdf(tmp_fpath) + del ds + return xr.open_dataset(tmp_fpath) From 98c30232b9e4ddfb37934d05c919be00eb799747 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:15:27 +0200 Subject: [PATCH 5/8] remove depth axis reversal now using depth='elevation' in copernicusmarine.open_dataset() --- src/virtualship/instruments/base.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index f281b899..18c175d3 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -231,10 +231,6 @@ def _generate_fieldset(self) -> parcels.FieldSet: ) field_var_name = var - # # negate depth and reindex (to suit Parcels XGrid strictly increasing depth convention) - # ds["depth"] = -ds["depth"] - # ds = ds.reindex(depth=ds["depth"][::-1]) - # TODO: to be removed when Parcels #2746 is merged (i.e. https://github.com/Parcels-code/Parcels/pull/2746) ds = ds.fillna(0) From c7f2c10e7931a8de096cfdd1c74296154dcba7c5 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 21 Jul 2026 16:04:16 +0200 Subject: [PATCH 6/8] tidy up todo --- src/virtualship/instruments/drifter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/virtualship/instruments/drifter.py b/src/virtualship/instruments/drifter.py index 6319831e..3b52dc71 100644 --- a/src/virtualship/instruments/drifter.py +++ b/src/virtualship/instruments/drifter.py @@ -89,7 +89,7 @@ def __init__(self, expedition, from_data): **sensor_variables, } # advection variables (U and V) are always required for drifter simulation; sensor variables come from config fetch_spec = FetchSpec( - latlon_buffer=30.0, # TODO: generous buffer to limit tmp file size download, can potentially be removed in the future as and when Parcels streaming performance improves (see #358) + latlon_buffer=30.0, # TODO: generous buffer to reduce tmp file footprint, can potentially be removed in the future as/when Parcels streaming performance improves (see #358) time_buffer=expedition.instruments_config.drifter_config.lifetime.total_seconds() / (24 * 3600), # [days] depth_min=abs( From e1c51222cdda7b6674d16db99443629fd4be155a Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:59:08 +0200 Subject: [PATCH 7/8] small update: fetch spec to match changes implemented in #359 --- tests/instruments/test_base.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/instruments/test_base.py b/tests/instruments/test_base.py index 1b090cb9..3ca4fc94 100644 --- a/tests/instruments/test_base.py +++ b/tests/instruments/test_base.py @@ -89,25 +89,28 @@ def test_execute_calls_simulate(monkeypatch): dummy.simulate.assert_called_once() -def test_get_spec_value_buffer_and_limit(): +def test_fetch_spec_applied_to_instrument(): + """FetchSpec values are correctly stored on the instrument.""" mock_waypoint = MagicMock() mock_waypoint.location.latitude = 1.0 mock_waypoint.location.longitude = 2.0 mock_schedule = MagicMock() mock_schedule.waypoints = [mock_waypoint] + fetch_spec = FetchSpec(latlon_buffer=5.0, depth_min=10.0) dummy = DummyInstrument( expedition=MagicMock(schedule=mock_schedule), variables={"A": "a"}, add_bathymetry=False, allow_time_extrapolation=False, verbose_progress=False, - spacetime_buffer_size={"latlon": 5.0}, - limit_spec={"depth_min": 10.0}, + fetch_spec=fetch_spec, from_data=None, ) - assert dummy._get_spec_value("buffer", "latlon", 0.0) == 5.0 - assert dummy._get_spec_value("limit", "depth_min", None) == 10.0 - assert dummy._get_spec_value("buffer", "missing", 42) == 42 + assert dummy.fetch_spec.latlon_buffer == 5.0 + assert dummy.fetch_spec.depth_min == 10.0 + # unset values use dataclass defaults + assert dummy.fetch_spec.time_buffer == 0.0 + assert dummy.fetch_spec.depth_max is None def test_generate_fieldset_combines_fields(monkeypatch): From 710acabccc46449cb76d9c22735e22a5de609da2 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:05:13 +0200 Subject: [PATCH 8/8] add test for _via_tmp_ds --- tests/instruments/test_base.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/instruments/test_base.py b/tests/instruments/test_base.py index 3ca4fc94..93a38e90 100644 --- a/tests/instruments/test_base.py +++ b/tests/instruments/test_base.py @@ -1,6 +1,7 @@ from unittest.mock import MagicMock, patch import pytest +import xarray as xr from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.types import InstrumentType @@ -113,6 +114,21 @@ def test_fetch_spec_applied_to_instrument(): assert dummy.fetch_spec.depth_max is None +def test_via_tmp_ds_roundtrip(): + """_via_tmp_ds writes to a tmp file and re-opens it.""" + ds = xr.Dataset( + {"temperature": (["x", "y"], [[1.0, 2.0], [3.0, 4.0]])}, + coords={"x": [0, 1], "y": [10, 20]}, + ) + result = Instrument._via_tmp_ds(ds) + + assert isinstance(result, xr.Dataset) + assert "temperature" in result + assert ( + result is not ds + ) # result is new object loaded from tmp file, not the original + + def test_generate_fieldset_combines_fields(monkeypatch): mock_waypoint = MagicMock() mock_waypoint.location.latitude = 1.0