Skip to content

Refactor dataset retrieval specs #359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
j-atkins wants to merge 2 commits into
base: migrate-v4
Choose a base branch
from
Open

Refactor dataset retrieval specs #359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f60d769
small refactor: consolidate spacetime_buffer_size and limit_spec dict…
j-atkins Jul 21, 2026
e0187b1
to FetchSpec dataclass
j-atkins Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions src/virtualship/instruments/adcp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -61,18 +61,14 @@ 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

super().__init__(
expedition,
variables,
add_bathymetry=False,
allow_time_extrapolation=True,
verbose_progress=False,
spacetime_buffer_size=None,
limit_spec=limit_spec,
fetch_spec=FetchSpec(),
from_data=from_data,
)

Expand Down
16 changes: 6 additions & 10 deletions src/virtualship/instruments/argo_float.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -253,23 +253,19 @@ 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 = {
"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]
}
limit_spec = {
"spatial": True, # spatial limits; lat/lon constrained to waypoint locations + buffer
}
)

super().__init__(
expedition,
variables,
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,
)

Expand Down
37 changes: 19 additions & 18 deletions src/virtualship/instruments/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import abc
import collections
from dataclasses import dataclass
from datetime import timedelta
from itertools import pairwise
from pathlib import Path
Expand All @@ -28,6 +29,17 @@
from virtualship.models import Expedition


@dataclass
class FetchSpec:

@erikvansebille erikvansebille Jul 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this class need its own unit tests? Or is it tested sufficiently with the instrument tests?

"""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."""

Expand All @@ -50,8 +62,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: FetchSpec | None = None,
):
"""Initialise instrument."""
self.expedition = expedition
Expand All @@ -67,8 +78,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 or FetchSpec()

wp_lats, wp_lons = _get_waypoint_latlons(expedition.schedule.waypoints)
wp_times = [
Expand Down Expand Up @@ -152,12 +162,10 @@ def _get_copernicus_ds(
variable=var if not physical else None,
)

latlon_buffer = self._get_spec_value(
"buffer", "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)
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
Expand All @@ -176,8 +184,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",
)

Expand All @@ -190,7 +196,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.fetch_spec.time_buffer

for key in keys:
var = self.variables[key]
Expand Down Expand Up @@ -251,8 +257,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
8 changes: 2 additions & 6 deletions src/virtualship/instruments/ctd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -142,18 +142,14 @@ 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

super().__init__(
expedition,
variables,
add_bathymetry=True,
allow_time_extrapolation=True,
verbose_progress=False,
spacetime_buffer_size=None,
limit_spec=limit_spec,
fetch_spec=FetchSpec(),
from_data=from_data,
)

Expand Down
20 changes: 8 additions & 12 deletions src/virtualship/instruments/drifter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -88,29 +88,25 @@ 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,
"time": expedition.instruments_config.drifter_config.lifetime.total_seconds()
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]
}
limit_spec = {
"spatial": False, # no spatial limits; generate global fieldset
"depth_min": abs(
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,
variables,
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,
)

Expand Down
12 changes: 2 additions & 10 deletions src/virtualship/instruments/ship_underwater_st.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -67,22 +67,14 @@ def __init__(self, expedition, from_data):
variables = (
expedition.instruments_config.ship_underwater_st_config.active_variables()
)
spacetime_buffer_size = {
"latlon": 0.25, # [degrees]
"time": 0.0, # [days]
}
limit_spec = {
"spatial": True
} # spatial limits; lat/lon constrained to waypoint locations + buffer

super().__init__(
expedition,
variables,
add_bathymetry=False,
allow_time_extrapolation=True,
verbose_progress=False,
spacetime_buffer_size=spacetime_buffer_size,
limit_spec=limit_spec,
fetch_spec=FetchSpec(),
from_data=from_data,
)

Expand Down
8 changes: 2 additions & 6 deletions src/virtualship/instruments/xbt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -95,18 +95,14 @@ 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

super().__init__(
expedition,
variables,
add_bathymetry=True,
allow_time_extrapolation=True,
verbose_progress=False,
spacetime_buffer_size=None,
limit_spec=limit_spec,
fetch_spec=FetchSpec(),
from_data=from_data,
)

Expand Down
Loading