From a8a1e0a7ada5f3d93c7812a1104869e8a183fa66 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:19:29 +0200 Subject: [PATCH 01/39] move verify methods up to Expedition model; give method names reflecting whether schedule or instr config; use more helper functions --- src/virtualship/cli/_plan.py | 10 +- src/virtualship/cli/_run.py | 6 +- src/virtualship/models/expedition.py | 220 ++++++++++++++++----------- tests/cli/test_run.py | 5 +- tests/expedition/test_expedition.py | 40 +++-- 5 files changed, 156 insertions(+), 125 deletions(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index 5dd3e9ac..9e60a32b 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -1124,20 +1124,24 @@ def save_pressed(self) -> None: "Some waypoint entries are invalid or incomplete:\n\n" + "\n".join(waypoint_errors) ) + ship_speed = self.get_ship_speed(editor) - schedule = editor.expedition.schedule - schedule.verify( - ship_speed, editor.expedition.instruments_config, ignore_land_test=True + editor.expedition.verify_schedule( + ignore_land_test=True, ship_speed=ship_speed ) + if editor.save_changes(): self.notify( "Changes saved successfully", severity="information", timeout=20 ) + # check for incomplete ports and warn the user, but allow save to continue + schedule = editor.expedition.schedule if not ( schedule.departure_port.is_in_use and schedule.arrival_port.is_in_use ): self.notify(INCOMPLETE_PORT_MSG, severity="warning", timeout=20) + except Exception as e: self.notify( f"*** Error saving changes ***:\n\n{e}\n", diff --git a/src/virtualship/cli/_run.py b/src/virtualship/cli/_run.py index a73b275d..d9042e14 100644 --- a/src/virtualship/cli/_run.py +++ b/src/virtualship/cli/_run.py @@ -92,7 +92,7 @@ def _run( ) # verify instruments_config file is consistent with schedule - expedition.instruments_config.verify(expedition) + expedition.verify_instruments_config() # load last checkpoint checkpoint = _load_checkpoint(expedition_dir) @@ -103,9 +103,7 @@ def _run( print("\n---- WAYPOINT VERIFICATION ----") - schedule.verify( - expedition.ship_config.ship_speed_knots, - expedition.instruments_config, + expedition.verify_schedule( from_data=Path(from_data) if from_data else None, ) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 60e3c4fb..1c1d27c8 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -1,6 +1,7 @@ from __future__ import annotations import itertools +from collections.abc import Collection from datetime import datetime, timedelta from pathlib import Path from typing import ClassVar, cast @@ -51,7 +52,7 @@ def from_yaml(cls, file_path: str) -> Expedition: data = yaml.safe_load(file) return Expedition(**data) - def get_instruments(self) -> set[InstrumentType]: + def get_instruments(self) -> list[InstrumentType]: """Return a set of unique InstrumentType enums used in the expedition.""" instruments_in_expedition = [] @@ -76,6 +77,100 @@ def get_instruments(self) -> set[InstrumentType]: "Underway instrument config attribute(s) are missing from YAML. Must be Config object or None." ) from e + def verify_instruments_config(self) -> None: + """Check the instrument configurations cover exactly the instruments this expedition deploys.""" + self.instruments_config.verify(self.get_instruments()) + + def verify_schedule( + self, + ignore_land_test: bool = False, + *, + ship_speed: float | None = None, + from_data: Path | None = None, + ) -> None: + """ + Verify the feasibility and correctness of the expedition's schedule. + + `ship_speed` overrides the configured speed, for checking a prospective speed that has not been saved yet (e.g. in `plan` app). + """ + print("\nVerifying route... ") + + if ship_speed is None: + ship_speed = self.ship_config.ship_speed_knots + + # waypoints excluding any inactive placeholder departure/arrival ports + wps_in_use = self.schedule._get_wps_in_use() + + self.schedule._check_structure(wps_in_use) + + if not ignore_land_test: + self.schedule._check_in_water(from_data) + + # check that ship will arrive on time at each waypoint (in case no unexpected event happen) + overrun = self._first_overrun(wps_in_use, ship_speed) + + if overrun is not None: + wp_i, wp_next, arrival_time = overrun + + # TODO: add messaging of stationkeeping time to the error message, e.g. how much each instrument is taking... + raise ScheduleError( + f"Waypoint planning is not valid: would arrive too late at {self._describe_waypoint(wp_i, wp_next)}. " + f"Location: {wp_next.location} Time: {wp_next.time}. " + f"Currently projected to arrive at: {arrival_time}." + "\n\nHint: adding instruments may increase the amount of time spent stationary at a waypoints. " + "Have you ensured that your schedule includes sufficient time for taking measurements, e.g. CTD casts (in addition to the time it takes to sail between waypoints)?\n" + ) + + # finally, mark this schedule as verified (so that subsequent stages of the workflow can proceed without re-verifying) + self.schedule._verified = True + + print("... All good to go!") + + def _first_overrun( + self, + wps_in_use: list[Port | Waypoint], + ship_speed: float, + ) -> tuple[int, Port | Waypoint, datetime] | None: + """Project the ship along the schedule and return the first waypoint it reaches late.""" + time = wps_in_use[0].time + + # offset from wps_in_use indices to self.schedule.waypoints indices (a placeholder departure port is excluded from wps_in_use) + wps_in_use_offset = 0 if self.schedule.departure_port.is_in_use else 1 + + for wp_i, (wp, wp_next) in enumerate( + itertools.pairwise(wps_in_use), start=wps_in_use_offset + ): + stationkeeping_time = ( + wp.stationkeeping_time(self.instruments_config) + if isinstance(wp, Waypoint) + else timedelta() + ) + + time_to_reach = _calc_sail_time( + wp.location, + wp_next.location, + ship_speed, + projection, + )[0] + + arrival_time = time + time_to_reach + stationkeeping_time + + if wp_next.time is None: + time = arrival_time + elif arrival_time > wp_next.time: + return wp_i, wp_next, arrival_time + else: + time = wp_next.time + + return None + + def _describe_waypoint(self, wp_i: int, wp_next: Port | Waypoint) -> str: + """Name the waypoint reached after index `wp_i`, for use in user-facing messages.""" + if isinstance(wp_next, Port): + return "the final port of arrival" + # +1 to get next + return f"waypoint {_get_public_wp(wp_i + 1, self.schedule.waypoints)}" + def _annotate(self): """ Add port/waypoint comments/annotations to the expedition.yaml file. @@ -159,20 +254,9 @@ def arrival_port(self) -> Port: """Arrival port (always the last waypoint).""" return cast(Port, self.waypoints[-1]) - def verify( - self, - ship_speed: float, - instruments_config: InstrumentsConfig, - ignore_land_test: bool = False, - *, - from_data: Path | None = None, - ) -> None: - """Verify the feasibility and correctness of the schedule's waypoints.""" - print("\nVerifying route... ") - - # waypoints excluding any inactive placeholder departure/arrival ports - wps_in_use = self._get_wps_in_use() - + def _check_structure(self, wps_in_use: list[Port | Waypoint]) -> None: + """Check the schedule is structured properly: has waypoint(s) and times in order.""" + # has wps in use if not wps_in_use: raise ScheduleError( "Schedule has no active waypoints: at least one waypoint, or an " @@ -180,7 +264,6 @@ def verify( "must be provided." ) - # is the departure port in use or a placeholder (i.e. all None)? wp_str = "Departure port" if self.departure_port.is_in_use else "Waypoint 1" if wps_in_use[0].time is None: @@ -207,88 +290,40 @@ def verify( f"Waypoint(s) {', '.join(invalid_labels)}: each waypoint should be timed after all previous waypoints", ) - # check if all non-port waypoints are in water using bathymetry data + def _check_in_water(self, from_data: Path | None = None) -> None: + """Check non-port waypoints sit in water rather than on land.""" + try: + bathymetry_field = _get_bathy_data(from_data=from_data).bathymetry + except Exception as e: + raise ScheduleError( + f"Problem loading bathymetry data (used to verify waypoints are in water) directly via copernicusmarine. \n\n original message: {e}" + ) from e + land_waypoints = [] - if not ignore_land_test: + + for wp_i, wp in enumerate(self.waypoints): + if isinstance(wp, Port): + continue # ports are in harbour; skip bathymetry land check + public_wp = _get_public_wp(wp_i, self.waypoints) try: - bathymetry_field = _get_bathy_data(from_data=from_data).bathymetry + value = bathymetry_field.eval( + 0, # time + 0, # depth (surface) + wp.location.lat, + wp.location.lon, + ) + if value == 0.0 or (isinstance(value, float) and np.isnan(value)): + land_waypoints.append((public_wp, wp)) except Exception as e: raise ScheduleError( - f"Problem loading bathymetry data (used to verify waypoints are in water) directly via copernicusmarine. \n\n original message: {e}" + f"Waypoint #{public_wp} at location {wp.location} could not be evaluated against bathymetry data. \n\n Original error: {e}" ) from e - for wp_i, wp in enumerate(self.waypoints): - if isinstance(wp, Port): - continue # ports are in harbour; skip bathymetry land check - public_wp = _get_public_wp(wp_i, self.waypoints) - try: - value = bathymetry_field.eval( - 0, # time - 0, # depth (surface) - wp.location.lat, - wp.location.lon, - ) - if value == 0.0 or (isinstance(value, float) and np.isnan(value)): - land_waypoints.append((public_wp, wp)) - except Exception as e: - raise ScheduleError( - f"Waypoint #{public_wp} at location {wp.location} could not be evaluated against bathymetry data. \n\n Original error: {e}" - ) from e - - if len(land_waypoints) > 0: - raise ScheduleError( - f"The following waypoint(s) throw(s) error(s): {['#' + str(public_wp) + ' ' + str(wp) for (public_wp, wp) in land_waypoints]}\n\nINFO: They are likely on land (bathymetry data cannot be interpolated to their location(s)).\n" - ) - - # check that ship will arrive on time at each waypoint (in case no unexpected event happen) - time = wps_in_use[0].time - - # offset from wps_in_use indices to self.waypoints indices (a placeholder departure port is excluded from wps_in_use) - wps_in_use_offset = 0 if self.departure_port.is_in_use else 1 - - for wp_i, (wp, wp_next) in enumerate( - itertools.pairwise(wps_in_use), start=wps_in_use_offset - ): - stationkeeping_time = ( - wp.stationkeeping_time(instruments_config) - if isinstance(wp, Waypoint) - else timedelta() + if len(land_waypoints) > 0: + raise ScheduleError( + f"The following waypoint(s) throw(s) error(s): {['#' + str(public_wp) + ' ' + str(wp) for (public_wp, wp) in land_waypoints]}\n\nINFO: They are likely on land (bathymetry data cannot be interpolated to their location(s)).\n" ) - time_to_reach = _calc_sail_time( - wp.location, - wp_next.location, - ship_speed, - projection, - )[0] - - arrival_time = time + time_to_reach + stationkeeping_time - - if wp_next.time is None: - time = arrival_time - elif arrival_time > wp_next.time: - affected = ( - f"waypoint {_get_public_wp(wp_i + 1, self.waypoints)}" # +1 to get next - if not isinstance(wp_next, Port) - else "the final port of arrival" - ) - - # TODO: add messaging of stationkeeping time to the error message, e.g. how much each instrument is taking... - raise ScheduleError( - f"Waypoint planning is not valid: would arrive too late at {affected}. " - f"Location: {wp_next.location} Time: {wp_next.time}. " - f"Currently projected to arrive at: {arrival_time}." - "\n\nHint: adding instruments may increase the amount of time spent stationary at a waypoints. " - "Have you ensured that your schedule includes sufficient time for taking measurements, e.g. CTD casts (in addition to the time it takes to sail between waypoints)?\n" - ) - else: - time = wp_next.time - - # finally, mark this schedule as verified (so that subsequent stages of the workflow can proceed without re-verifying) - self._verified = True - - print("... All good to go!") - def _get_wps_in_use(self) -> list[Port | Waypoint]: """Return waypoints that are in use (i.e., have a specified time and location), i.e. excluding placeholder departure/arrival ports.""" start_slice = 0 if self.departure_port.is_in_use else 1 @@ -651,14 +686,13 @@ class InstrumentsConfig(pydantic.BaseModel): model_config = pydantic.ConfigDict(extra="forbid") - def verify(self, expedition: Expedition) -> None: + def verify(self, instruments_in_expedition: Collection[InstrumentType]) -> None: """ - Verify instrument configurations against the schedule. + Verify instrument configurations against the instruments actually being deployed. Removes instrument configs not present in the schedule and checks that all scheduled instruments are configured. Raises ConfigError if any scheduled instrument is missing a config. """ - instruments_in_expedition = expedition.get_instruments() instrument_config_map = { InstrumentType.ARGO_FLOAT: "argo_float_config", InstrumentType.DRIFTER: "drifter_config", diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py index 2124eb52..8c6d3990 100644 --- a/tests/cli/test_run.py +++ b/tests/cli/test_run.py @@ -52,10 +52,11 @@ def test_run(tmp_path, monkeypatch): monkeypatch.setattr("virtualship.cli._run.simulate_schedule", _simulate_schedule) monkeypatch.setattr( - "virtualship.models.InstrumentsConfig.verify", lambda self, expedition: None + "virtualship.models.InstrumentsConfig.verify", lambda self, instruments: None ) monkeypatch.setattr( - "virtualship.models.Schedule.verify", lambda self, *args, **kwargs: None + "virtualship.models.Expedition.verify_schedule", + lambda self, *args, **kwargs: None, ) monkeypatch.setattr( diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index b40f6dd0..a7465cbb 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -29,6 +29,15 @@ expedition_dir = Path("expedition_dir") +def _with_schedule(base_expedition, schedule) -> Expedition: + """An expedition using the given schedule, with the shared ship and instrument configs.""" + return Expedition( + schedule=schedule, + instruments_config=base_expedition.instruments_config, + ship_config=base_expedition.ship_config, + ) + + @pytest.fixture def base_expedition(): """Shared expedition instance loaded directly from expedition_dir.""" @@ -51,11 +60,7 @@ def test_import_export_expedition(tmpdir, base_expedition) -> None: def test_verify_schedule(base_expedition) -> None: schedule = base_expedition.schedule - schedule.verify( - base_expedition.ship_config.ship_speed_knots, - base_expedition.instruments_config, - ignore_land_test=True, - ) + base_expedition.verify_schedule(ignore_land_test=True) assert schedule._verified, ( "Schedule should be marked as verified after successful verification." @@ -167,11 +172,8 @@ def test_verify_on_land(base_expedition): ScheduleError, match=r"The following waypoint\(s\) throw\(s\) error\(s\):", ): - schedule.verify( - base_expedition.ship_config.ship_speed_knots, - base_expedition.instruments_config, - ignore_land_test=False, - from_data=None, + _with_schedule(base_expedition, schedule).verify_schedule( + ignore_land_test=False, from_data=None ) @@ -269,11 +271,7 @@ def test_verify_on_land(base_expedition): def test_verify_schedule_errors(base_expedition, waypoints: list, error, match) -> None: with pytest.raises(error, match=match): schedule = Schedule(waypoints=waypoints) - schedule.verify( - base_expedition.ship_config.ship_speed_knots, - base_expedition.instruments_config, - ignore_land_test=True, - ) + _with_schedule(base_expedition, schedule).verify_schedule(ignore_land_test=True) def test_verify_schedule_ports_only(base_expedition) -> None: @@ -284,11 +282,7 @@ def test_verify_schedule_ports_only(base_expedition) -> None: Port(location=Location(1, 0), time=datetime(2022, 1, 2, 0, 0, 0)), ] ) - schedule.verify( - base_expedition.ship_config.ship_speed_knots, - base_expedition.instruments_config, - ignore_land_test=True, - ) + _with_schedule(base_expedition, schedule).verify_schedule(ignore_land_test=True) assert schedule._verified, "Schedule with only active ports should be accepted." @@ -344,11 +338,11 @@ def missing_instrument_config(request, expedition): def test_verify_instruments_config(expedition) -> None: - expedition.instruments_config.verify(expedition) + expedition.verify_instruments_config() def test_verify_instruments_config_no_instrument(expedition, expedition_no_xbt) -> None: - expedition.instruments_config.verify(expedition_no_xbt) + expedition.instruments_config.verify(expedition_no_xbt.get_instruments()) def test_verify_instruments_config_errors( @@ -356,7 +350,7 @@ def test_verify_instruments_config_errors( ) -> None: instruments_config, match = missing_instrument_config with pytest.raises(InstrumentsConfigError, match=match): - instruments_config.verify(expedition) + instruments_config.verify(expedition.get_instruments()) def test_all_instrument_configs_use_mixin(expedition): From 8d35fc7ffea7cb8b8d1c6f531677d8a4d175a7a9 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:50:01 +0200 Subject: [PATCH 02/39] move sensor_kernels check to register_instrument rather than __init__subclass__ --- src/virtualship/instruments/base.py | 13 +------------ src/virtualship/utils.py | 18 +++++++++++++----- tests/instruments/test_base.py | 20 ++++++++++++++++---- tests/test_utils.py | 14 ++++++++++---- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index ff974996..bac96342 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -2,7 +2,6 @@ import abc import collections -import inspect import itertools from dataclasses import dataclass from datetime import datetime, timedelta @@ -90,19 +89,9 @@ class Instrument(abc.ABC): """Base class for instruments and their simulation.""" # all instruments have sensor_kernels dict, mapping SensorType to sampling kernel + # (required of every registered instrument; checked by @register_instrument) sensor_kernels: ClassVar[dict[SensorType, collections.abc.Callable]] - def __init_subclass__(cls, **kwargs: object) -> None: - """Ensure concrete instrument subclasses define required class attributes.""" - super().__init_subclass__(**kwargs) - if inspect.isabstract(cls): - return - - if "sensor_kernels" not in cls.__dict__: - raise TypeError( - f"Instrument subclass '{cls.__name__}' must define 'sensor_kernels' as a class attribute." - ) - def __init__( self, expedition: Expedition, diff --git a/src/virtualship/utils.py b/src/virtualship/utils.py index aeca9082..c6dd4762 100644 --- a/src/virtualship/utils.py +++ b/src/virtualship/utils.py @@ -2,6 +2,7 @@ import glob import hashlib +import inspect import json import re import sys @@ -129,12 +130,19 @@ def register_instrument(instrument_type): + """Register a concrete instrument class, refusing if doesn't meet the full Instrument contract.""" + def decorator(cls): - INSTRUMENT_CLASS_MAP[instrument_type] = cls - if hasattr(cls, "sensor_kernels"): # derive supported kernels from class attr - SUPPORTED_SENSORS_MAP[instrument_type] = frozenset( - cls.sensor_kernels.keys() + if inspect.isabstract(cls): + missing = ", ".join(sorted(cls.__abstractmethods__)) + raise TypeError(f"Instrument '{cls.__name__}' must implement: {missing}.") + if "sensor_kernels" not in vars(cls): + raise TypeError( + f"Instrument '{cls.__name__}' must define 'sensor_kernels' as a class attribute." ) + + INSTRUMENT_CLASS_MAP[instrument_type] = cls + SUPPORTED_SENSORS_MAP[instrument_type] = frozenset(cls.sensor_kernels) return cls return decorator @@ -150,7 +158,7 @@ def get_supported_sensors(instrument_type): if supported is None: raise KeyError( f"No supported sensors registered for {instrument_type!r}. " - f"Does the instrument class define a `sensor_kernels` attribute?" + f"Is the instrument class decorated with @register_instrument?" ) return supported diff --git a/tests/instruments/test_base.py b/tests/instruments/test_base.py index 0d5979f8..95366321 100644 --- a/tests/instruments/test_base.py +++ b/tests/instruments/test_base.py @@ -19,7 +19,11 @@ from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.expedition import SensorConfig -from virtualship.utils import MAX_CACHE_BYTES, get_instrument_class +from virtualship.utils import ( + MAX_CACHE_BYTES, + get_instrument_class, + register_instrument, +) # ============================================================================= # Fixtures @@ -398,12 +402,20 @@ def test_load_input_data_error(mock_expedition, monkeypatch): dummy.load_input_data() -def test_instrument_subclass_without_sensor_kernels_error(): - """Defining a concrete Instrument subclass without sensor_kernels raises TypeError.""" +@pytest.fixture +def isolated_registry(monkeypatch): + """Keep test registrations out of the real instrument registry.""" + monkeypatch.setattr("virtualship.utils.INSTRUMENT_CLASS_MAP", {}) + monkeypatch.setattr("virtualship.utils.SUPPORTED_SENSORS_MAP", {}) + + +def test_register_instrument_without_sensor_kernels_error(isolated_registry): + """Registering an instrument without sensor_kernels raises TypeError.""" with pytest.raises(TypeError, match="sensor_kernels"): + @register_instrument(InstrumentType.CTD) class ErrorInstrument(Instrument): - def simulate(self, data_dir, measurements, out_path): + def simulate(self, measurements, out_path): pass diff --git a/tests/test_utils.py b/tests/test_utils.py index 788d64fb..cee8f068 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -9,6 +9,7 @@ from parcels import FieldSet, ParticleClass, Variable import virtualship.utils +from virtualship.instruments.base import Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.expedition import Expedition, Port, SensorConfig, Waypoint @@ -35,10 +36,13 @@ def expedition(tmp_file): @pytest.fixture def dummy_instrument(): - class DummyInstrument: - pass + class DummyInstrument(Instrument): + sensor_kernels = {} # noqa - return DummyInstrument() + def simulate(self, measurements, out_path): + pass + + return DummyInstrument @pytest.fixture @@ -69,9 +73,11 @@ def fake_open_dataset(*args, **kwargs): yield -def test_instrument_registry_updates(dummy_instrument): +def test_instrument_registry_updates(dummy_instrument, monkeypatch): from virtualship import utils + monkeypatch.setattr(utils, "INSTRUMENT_CLASS_MAP", {}) + monkeypatch.setattr(utils, "SUPPORTED_SENSORS_MAP", {}) utils.register_instrument("DUMMY_TYPE")(dummy_instrument) assert utils.INSTRUMENT_CLASS_MAP["DUMMY_TYPE"] is dummy_instrument From 69d429a4d65a190058e72d32e18bfec0fb64f6e2 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:52:55 +0200 Subject: [PATCH 03/39] add deceleration and acceleration times to ship config --- src/virtualship/models/expedition.py | 22 ++++++++++++++++++- src/virtualship/static/expedition.yaml | 2 ++ .../expedition/expedition_dir/expedition.yaml | 2 ++ tests/expedition/test_simulate_schedule.py | 16 +++++++++----- .../make_realistic/problems/test_simulator.py | 6 ++++- 5 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 1c1d27c8..06fc8eef 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -220,9 +220,29 @@ class ShipConfig(pydantic.BaseModel): ship_speed_knots: float = pydantic.Field(gt=0.0) + deceleration_time: timedelta = pydantic.Field( + serialization_alias="deceleration_time_minutes", + validation_alias="deceleration_time_minutes", + ge=timedelta(), + ) + acceleration_time: timedelta = pydantic.Field( + serialization_alias="acceleration_time_minutes", + validation_alias="acceleration_time_minutes", + ge=timedelta(), + ) + # TODO: room here for adding more ship config options in future PRs (e.g. max_days_at_sea)... - model_config = pydantic.ConfigDict(extra="forbid") + model_config = pydantic.ConfigDict(extra="forbid", populate_by_name=True) + + @pydantic.field_serializer("deceleration_time", "acceleration_time") + def _serialize_minutes(self, value: timedelta, _info) -> float: + return value.total_seconds() / 60.0 + + @pydantic.field_validator("deceleration_time", "acceleration_time", mode="before") + @classmethod + def _validate_minutes(cls, value: int | float | timedelta) -> timedelta: + return _validate_numeric_to_timedelta(value, "minutes") class Schedule(pydantic.BaseModel): diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index 1e201543..0ec21f74 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -100,3 +100,5 @@ schedule: time: 1998-01-07 05:00:00 ship_config: ship_speed_knots: 10.0 + deceleration_time_minutes: 15.0 + acceleration_time_minutes: 15.0 diff --git a/tests/expedition/expedition_dir/expedition.yaml b/tests/expedition/expedition_dir/expedition.yaml index 630d7342..f4a271df 100644 --- a/tests/expedition/expedition_dir/expedition.yaml +++ b/tests/expedition/expedition_dir/expedition.yaml @@ -53,3 +53,5 @@ instruments_config: period_minutes: 5.0 ship_config: ship_speed_knots: 10.0 + deceleration_time_minutes: 15.0 + acceleration_time_minutes: 15.0 diff --git a/tests/expedition/test_simulate_schedule.py b/tests/expedition/test_simulate_schedule.py index f33e3b52..5e7400df 100644 --- a/tests/expedition/test_simulate_schedule.py +++ b/tests/expedition/test_simulate_schedule.py @@ -11,13 +11,15 @@ from virtualship.models import Expedition, Location, Schedule, Waypoint from virtualship.models.expedition import Port +TEST_EXPEDITION_YAML = "expedition_dir/expedition.yaml" + def test_simulate_schedule_feasible() -> None: """Test schedule with two waypoints that can be reached within time is OK.""" base_time = datetime.strptime("2022-01-01T00:00:00", "%Y-%m-%dT%H:%M:%S") projection = pyproj.Geod(ellps="WGS84") - expedition = Expedition.from_yaml("expedition_dir/expedition.yaml") + expedition = Expedition.from_yaml(TEST_EXPEDITION_YAML) expedition.ship_config.ship_speed_knots = 10.0 expedition.schedule = Schedule( waypoints=[ @@ -37,9 +39,7 @@ def test_simulate_schedule_feasible() -> None: def test_time_in_minutes_in_ship_schedule() -> None: """Test whether the pydantic serializer picks up the time *in minutes* in the ship schedule.""" - instruments_config = Expedition.from_yaml( - "expedition_dir/expedition.yaml" - ).instruments_config + instruments_config = Expedition.from_yaml(TEST_EXPEDITION_YAML).instruments_config assert instruments_config.adcp_config.period == timedelta(minutes=5) assert instruments_config.ctd_config.stationkeeping_time == timedelta(minutes=50) assert instruments_config.argo_float_config.stationkeeping_time == timedelta( @@ -50,13 +50,17 @@ def test_time_in_minutes_in_ship_schedule() -> None: ) assert instruments_config.ship_underwater_st_config.period == timedelta(minutes=5) + ship_config = Expedition.from_yaml(TEST_EXPEDITION_YAML).ship_config + assert ship_config.deceleration_time == timedelta(minutes=15) + assert ship_config.acceleration_time == timedelta(minutes=15) + def test_ship_path_inside_domain() -> None: """Test that the ship path (here represented by underway ADCP measurement sites) is inside the domain defined by the waypoints (which determines the fieldset bounds).""" base_time = datetime.strptime("2022-01-01T00:00:00", "%Y-%m-%dT%H:%M:%S") projection = pyproj.Geod(ellps="WGS84") - expedition = Expedition.from_yaml("expedition_dir/expedition.yaml") + expedition = Expedition.from_yaml(TEST_EXPEDITION_YAML) expedition.ship_config.ship_speed_knots = 10.0 wp1 = Location(-63.0, -57.7) # most southern @@ -123,7 +127,7 @@ def test_does_not_simulate_unverified(): base_time = datetime.strptime("2022-01-01T00:00:00", "%Y-%m-%dT%H:%M:%S") projection = pyproj.Geod(ellps="WGS84") - expedition = Expedition.from_yaml("expedition_dir/expedition.yaml") + expedition = Expedition.from_yaml(TEST_EXPEDITION_YAML) expedition.ship_config.ship_speed_knots = 10.0 expedition.schedule = Schedule( waypoints=[ diff --git a/tests/make_realistic/problems/test_simulator.py b/tests/make_realistic/problems/test_simulator.py index 6432185d..5be80dd6 100644 --- a/tests/make_realistic/problems/test_simulator.py +++ b/tests/make_realistic/problems/test_simulator.py @@ -57,7 +57,11 @@ def _make_simple_expedition( schedule = Schedule(waypoints=waypoints) instruments = InstrumentsConfig() - ship = ShipConfig(ship_speed_knots=10.0) + ship = ShipConfig( + ship_speed_knots=10.0, + deceleration_time_minutes=15.0, + acceleration_time_minutes=15.0, + ) return Expedition( schedule=schedule, instruments_config=instruments, ship_config=ship ) From 2ba6f9ea669f97c4a3a111f70713bd4c910d02b8 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:17:08 +0200 Subject: [PATCH 04/39] add option and use bathymetry data (accessed in simulation) instead of the waypoint's sea_depth_meter field --- src/virtualship/models/expedition.py | 17 ++++++++++++----- tests/expedition/test_expedition.py | 11 +++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 1bd6b21d..c08f7505 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -166,20 +166,26 @@ def _describe_waypoint(self, wp_i: int, wp_next: Port | Waypoint) -> str: # +1 to get next return f"waypoint {_get_public_wp(wp_i + 1, self.schedule.waypoints)}" - def stationkeeping_time(self, waypoint: Port | Waypoint) -> timedelta: + def stationkeeping_time( + self, + waypoint: Port | Waypoint, + bathymetry: float | None = None, + ) -> timedelta: """ Calculate how long the ship is stationary at waypoint. Stationkeeping time = ship deceleration time + deployment time + ship acceleration time. Assumes insturments are deployed concurrently, so the longest deployment time is used. Empty waypoints (no instruments) cost nothing/ship sails through without slowing. + Optional `bathymetry` (measured seabed depth from simulation fieldset) overrides the waypoint's sea_depth_meter (estimated from MFP). """ instruments = self._wp_instruments(waypoint) if not instruments: return timedelta() deployment_time = max( - self._deployment_time(instrument, waypoint) for instrument in instruments + self._deployment_time(instrument, waypoint, bathymetry) + for instrument in instruments ) return ( @@ -192,15 +198,16 @@ def _deployment_time( self, instrument: InstrumentType, waypoint: Waypoint, + bathymetry: float | None = None, ) -> timedelta: """Time an instrument holds the ship on station.""" config = self.instruments_config.config_for(instrument) if config is None: return timedelta() - return get_instrument_class(instrument).deployment_time( - config, waypoint.sea_depth_meter - ) + # a measured seabed depth (from simulation fieldset), when given, stands in for the waypoint's own estimate + seabed = bathymetry if bathymetry is not None else waypoint.sea_depth_meter + return get_instrument_class(instrument).deployment_time(config, seabed) @staticmethod def _wp_instruments(waypoint: Port | Waypoint) -> list[InstrumentType]: diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index d44e8cd5..901c2139 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -130,6 +130,17 @@ def test_stationkeeping_time_capped_by_sea_depth(expedition) -> None: ) +def test_stationkeeping_time_bathymetry_overrides_waypoint_estimate(expedition) -> None: + """A measured seabed depth (from simulation fieldset) is used in place of the waypoint's estimate from MFP.""" + waypoint = Waypoint( + location=Location(0, 0), instrument=InstrumentType.CTD, sea_depth_meter=-100.0 + ) + + assert expedition.stationkeeping_time( + waypoint, bathymetry=-800.0 + ) > expedition.stationkeeping_time(waypoint) + + def test_stationkeeping_time_single_instrument(expedition) -> None: """A bare (non-list) InstrumentType value is handled the same as a single element list.""" bare = Waypoint( From f5f59ec16a3138d3ee72a2e40b0f786ebafa0811 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:57:14 +0200 Subject: [PATCH 05/39] enforce each instrument has an abstract deployment_time method (CTD dependent on depth); add winch_speed to CTD config --- src/virtualship/cli/_plan.py | 1 + src/virtualship/instruments/argo_float.py | 5 ++++ src/virtualship/instruments/base.py | 16 ++++++++++++- src/virtualship/instruments/ctd.py | 24 +++++++++++++++---- src/virtualship/instruments/drifter.py | 5 ++++ src/virtualship/instruments/xbt.py | 5 ++++ src/virtualship/models/expedition.py | 4 ++++ src/virtualship/static/expedition.yaml | 1 + tests/cli/test_plan.py | 3 ++- .../expedition/expedition_dir/expedition.yaml | 1 + tests/instruments/test_base.py | 23 +++++++++++++++++- tests/instruments/test_ctd.py | 6 +++++ tests/test_utils.py | 4 ++++ 13 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index 9e60a32b..fbe9b194 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -232,6 +232,7 @@ def _raise_logged_unexpected(e: Exception, path, context_message: str): "attributes": [ {"name": "max_depth_meter"}, {"name": "min_depth_meter"}, + {"name": "winch_speed_meter_per_second"}, {"name": "stationkeeping_time", "minutes": True}, ], }, diff --git a/src/virtualship/instruments/argo_float.py b/src/virtualship/instruments/argo_float.py index 8f4a4a50..b95d612b 100644 --- a/src/virtualship/instruments/argo_float.py +++ b/src/virtualship/instruments/argo_float.py @@ -263,6 +263,11 @@ def __init__(self, expedition, from_data): from_data=from_data, ) + @classmethod + def deployment_time(cls, config, seabed_depth: float | None) -> timedelta: + """Released and left behind, so no additional stationkeeping time.""" + return timedelta() + def simulate(self, measurements, out_path) -> None: """Simulate Argo float measurements.""" DT = 60.0 * 5 # dt of Argo float simulation integrator [seconds] diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index bac96342..d37597f1 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -34,7 +34,7 @@ if TYPE_CHECKING: from virtualship.instruments.sensors import SensorType - from virtualship.models import Expedition, Waypoint + from virtualship.models import Expedition, Waypoint, _InstrumentConfigMixin @dataclass(frozen=True) @@ -160,6 +160,13 @@ def load_input_data(self) -> parcels.FieldSet: return fieldset + @classmethod + @abc.abstractmethod + def deployment_time( + cls, config: _InstrumentConfigMixin, seabed_depth: float | None + ) -> timedelta: + """How long one deployment holds the ship on station, beyond slowing down and speeding up.""" + @abc.abstractmethod def simulate( self, @@ -402,6 +409,13 @@ def __post_init__(self): class UnderwayInstrument(Instrument): """Intermediate base class for underway instruments, which perform variable sampling without ParticleSets.""" + @classmethod + def deployment_time( + cls, config: _InstrumentConfigMixin, seabed_depth: float | None + ) -> timedelta: + """Samples continuously while the ship is under way, so never holds it at a waypoint.""" + return timedelta() + def _sample_underway( self, config_sensors: list, diff --git a/src/virtualship/instruments/ctd.py b/src/virtualship/instruments/ctd.py index 66c40a86..1174d4a0 100644 --- a/src/virtualship/instruments/ctd.py +++ b/src/virtualship/instruments/ctd.py @@ -148,9 +148,24 @@ def __init__(self, expedition, from_data): from_data=from_data, ) + @classmethod + def deployment_time(cls, config, seabed_depth: float | None) -> timedelta: + """Winched down to the seabed or its maximum depth, whichever is shallowest, and brought back up.""" + # depths are negative elevations, so the greater value is the shallower one + max_depth = ( + config.max_depth_meter + if seabed_depth is None + else max(config.max_depth_meter, seabed_depth) + ) + cast_distance = abs(max_depth - config.min_depth_meter) + return timedelta( + seconds=2 * cast_distance / config.winch_speed_meter_per_second + ) # 2 * for up and down + def simulate(self, measurements, out_path) -> None: """Simulate CTD measurements.""" - WINCH_SPEED = 1.0 # sink and rise speed in m/s + ctd_config = self.expedition.instruments_config.ctd_config + winch_speed = ctd_config.winch_speed_meter_per_second # sink/rise speed in m/s DT = 10.0 # dt of CTD simulation integrator OUTPUT_DT = timedelta(seconds=10) # output dt for CTD simulation @@ -180,13 +195,12 @@ def simulate(self, measurements, out_path) -> None: # CTD depth can not be too shallow, because kernel would break. # This shallow is not useful anyway, no need to support. - if not all([max_depth <= -DT * WINCH_SPEED for max_depth in max_depths]): + if not all([max_depth <= -DT * winch_speed for max_depth in max_depths]): raise ValueError( - f"CTD max_depth or bathymetry shallower than maximum {-DT * WINCH_SPEED}" + f"CTD max_depth or bathymetry shallower than maximum {-DT * winch_speed}" ) # build dynamic particle class from the active sensors - ctd_config = self.expedition.instruments_config.ctd_config _CTDParticle = build_particle_class_from_sensors( ctd_config.sensors, _CTD_NONSENSOR_VARIABLES ) @@ -201,7 +215,7 @@ def simulate(self, measurements, out_path) -> None: t=[ctd.spacetime.time for ctd in measurements], max_depth=max_depths, min_depth=[ctd.min_depth for ctd in measurements], - winch_speed=[WINCH_SPEED for _ in measurements], + winch_speed=[winch_speed for _ in measurements], ) # add initial conditions to sampling variables diff --git a/src/virtualship/instruments/drifter.py b/src/virtualship/instruments/drifter.py index 440fc5ee..833f1c27 100644 --- a/src/virtualship/instruments/drifter.py +++ b/src/virtualship/instruments/drifter.py @@ -100,6 +100,11 @@ def __init__(self, expedition, from_data): from_data=from_data, ) + @classmethod + def deployment_time(cls, config, seabed_depth: float | None) -> timedelta: + """Released and left behind, so no additional stationkeeping time.""" + return timedelta() + def simulate(self, measurements, out_path) -> None: """Simulate Drifter measurements.""" OUTPUT_DT = timedelta(hours=5) diff --git a/src/virtualship/instruments/xbt.py b/src/virtualship/instruments/xbt.py index a3fa33a4..e878465d 100644 --- a/src/virtualship/instruments/xbt.py +++ b/src/virtualship/instruments/xbt.py @@ -103,6 +103,11 @@ def __init__(self, expedition, from_data): from_data=from_data, ) + @classmethod + def deployment_time(cls, config, seabed_depth: float | None) -> timedelta: + """Dropped over the side and not recovered, so no additional stationkeeping times.""" + return timedelta() + def simulate(self, measurements, out_path) -> None: """Simulate XBT measurements.""" DT = 10.0 # dt of XBT simulation integrator diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 06fc8eef..8d8def8c 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -380,6 +380,9 @@ class Waypoint(pydantic.BaseModel): location: Location time: datetime | None = None instrument: InstrumentType | list[InstrumentType] | None = None + sea_depth_meter: float | None = pydantic.Field( + default=None, le=0.0 + ) # estimated depth from MFP, if available. If None, instrument deployment times costed against configured max depth, thus over-estimateing rather than under-estimating than under-estimating. @pydantic.field_serializer("instrument") def serialize_instrument(self, instrument): @@ -567,6 +570,7 @@ class CTDConfig(_InstrumentConfigMixin, pydantic.BaseModel): ) min_depth_meter: float = pydantic.Field(le=0.0) max_depth_meter: float = pydantic.Field(le=0.0) + winch_speed_meter_per_second: float = pydantic.Field(gt=0.0) sensors: list[SensorConfig] = pydantic.Field( default_factory=lambda: [ diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index 0ec21f74..2d62bf81 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -22,6 +22,7 @@ instruments_config: ctd_config: max_depth_meter: -2000.0 min_depth_meter: -11.0 + winch_speed_meter_per_second: 1.0 stationkeeping_time_minutes: 50.0 sensors: - TEMPERATURE diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 70d5b3f8..d799fdc0 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -282,7 +282,7 @@ async def test_sensor_toggle_saved_to_yaml(tmp_path): ) app = PlanApp(path=tmp_path) - async with app.run_test(size=(160, 120)) as pilot: + async with app.run_test(size=(160, 140)) as pilot: await pilot.pause(0.5) plan_screen = pilot.app.screen plan_screen.notify = MagicMock() @@ -408,6 +408,7 @@ async def test_sensor_initial_state_reflects_config(tmp_path): """Sensor switches should reflect the enabled sensors already stored in the config.""" ctd_config = CTDConfig( stationkeeping_time_minutes=50, + winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, sensors=[SensorConfig(sensor_type=SensorType.TEMPERATURE)], diff --git a/tests/expedition/expedition_dir/expedition.yaml b/tests/expedition/expedition_dir/expedition.yaml index f4a271df..20030d4f 100644 --- a/tests/expedition/expedition_dir/expedition.yaml +++ b/tests/expedition/expedition_dir/expedition.yaml @@ -44,6 +44,7 @@ instruments_config: ctd_config: max_depth_meter: -2000.0 min_depth_meter: -11.0 + winch_speed_meter_per_second: 1.0 stationkeeping_time_minutes: 50.0 drifter_config: depth_meter: -1.0 diff --git a/tests/instruments/test_base.py b/tests/instruments/test_base.py index 95366321..922630c6 100644 --- a/tests/instruments/test_base.py +++ b/tests/instruments/test_base.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from datetime import datetime +from datetime import datetime, timedelta from typing import ClassVar from unittest.mock import MagicMock, PropertyMock, patch @@ -153,6 +153,11 @@ class DummyInstrument(Instrument): sensor_kernels = {} # noqa + @classmethod + def deployment_time(cls, config, seabed_depth): + """Dummy deployment time for test.""" + return timedelta() + def simulate(self, measurements, out_path): """Dummy simulate implementation for test.""" self.simulate_called = True @@ -415,6 +420,22 @@ def test_register_instrument_without_sensor_kernels_error(isolated_registry): @register_instrument(InstrumentType.CTD) class ErrorInstrument(Instrument): + @classmethod + def deployment_time(cls, config, seabed_depth): + return timedelta() + + def simulate(self, measurements, out_path): + pass + + +def test_register_instrument_without_deployment_time_error(isolated_registry): + """Registering an instrument that leaves deployment_time unimplemented raises TypeError.""" + with pytest.raises(TypeError, match="deployment_time"): + + @register_instrument(InstrumentType.CTD) + class ErrorInstrument(Instrument): + sensor_kernels = {} # noqa + def simulate(self, measurements, out_path): pass diff --git a/tests/instruments/test_ctd.py b/tests/instruments/test_ctd.py index 76aa647b..f2c60344 100644 --- a/tests/instruments/test_ctd.py +++ b/tests/instruments/test_ctd.py @@ -54,6 +54,7 @@ class DummyExpedition: instruments_config = InstrumentsConfig( ctd_config=CTDConfig( stationkeeping_time_minutes=STATIONKEEPING_TIME, + winch_speed_meter_per_second=1.0, min_depth_meter=MIN_DEPTH, max_depth_meter=MAX_DEPTH, sensors=sensors, @@ -329,6 +330,7 @@ def test_ctd_sensor_config_active_variables() -> None: """active_variables() only returns variables for enabled sensors.""" config_both = CTDConfig( stationkeeping_time_minutes=50, + winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, sensors=[ @@ -340,6 +342,7 @@ def test_ctd_sensor_config_active_variables() -> None: config_temp_only = CTDConfig( stationkeeping_time_minutes=50, + winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, sensors=[ @@ -353,6 +356,7 @@ def test_ctd_sensor_config_yaml() -> None: """CTDConfig sensors survive YAML serialisation.""" config = CTDConfig( stationkeeping_time_minutes=50, + winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, sensors=[ @@ -437,6 +441,7 @@ def test_ctd_config_default_sensors(): """CTDConfig defaults to all supported sensors (phys + bgc).""" config = CTDConfig( stationkeeping_time_minutes=50, + winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, ) @@ -460,6 +465,7 @@ def test_ctd_config_unsupported_sensor_rejected(): with pytest.raises(pydantic.ValidationError, match="does not support"): CTDConfig( stationkeeping_time_minutes=50, + winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, sensors=[SensorConfig(sensor_type=SensorType.VELOCITY)], diff --git a/tests/test_utils.py b/tests/test_utils.py index cee8f068..8181ab08 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -39,6 +39,10 @@ def dummy_instrument(): class DummyInstrument(Instrument): sensor_kernels = {} # noqa + @classmethod + def deployment_time(cls, config, seabed_depth): + return datetime.timedelta() + def simulate(self, measurements, out_path): pass From 136304aabc667cb57bf7b72dc2d1c51379e79d62 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:18:39 +0200 Subject: [PATCH 06/39] warn when using bathymetry would break and otherwise valid schedule --- src/virtualship/models/expedition.py | 46 +++++++++++++++++---- tests/expedition/test_expedition.py | 61 ++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index c08f7505..6ed9a257 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -1,6 +1,7 @@ from __future__ import annotations import itertools +import warnings from collections.abc import Collection from datetime import datetime, timedelta from pathlib import Path @@ -102,8 +103,9 @@ def verify_schedule( self.schedule._check_structure(wps_in_use) - if not ignore_land_test: - self.schedule._check_in_water(from_data) + measured_depths = ( + {} if ignore_land_test else self.schedule._check_in_water(from_data) + ) # check that ship will arrive on time at each waypoint (in case no unexpected event happen) overrun = self._first_overrun(wps_in_use, ship_speed) @@ -115,12 +117,35 @@ def verify_schedule( raise ScheduleError( f"Waypoint planning is not valid: would arrive too late at {self._describe_waypoint(wp_i, wp_next)}. " f"Location: {wp_next.location} Time: {wp_next.time}. " - f"Currently projected to arrive at: {arrival_time}." + f"Currently projected to arrive at: {arrival_time.replace(microsecond=0)}." "\n\nHint: adding instruments may increase the amount of time spent stationary at a waypoints. " "Have you ensured that your schedule includes sufficient time for taking measurements, e.g. CTD casts (in addition to the time it takes to sail between waypoints)?\n" ) - # finally, mark this schedule as verified (so that subsequent stages of the workflow can proceed without re-verifying) + # the schedule works using the planned depths (e.g. from MFP), check it would also hold against the measured seabed (e.g. from bathy fieldset) + # warn when bringing in the measured sea depth would break the schedule + if measured_depths: + measured_overrun = self._first_overrun( + wps_in_use, ship_speed, measured_depths + ) + if measured_overrun is not None: + wp_i, wp_next, arrival_time = measured_overrun + # the deployment that overruns happens at the waypoint before the late arrival + wp = self.schedule.waypoints[wp_i] + wp_name = f"waypoint {_get_public_wp(wp_i, self.schedule.waypoints)}" + actual_depth = measured_depths[id(wp)] + warnings.warn( + f"The planned sea depth at {wp_name} (sea_depth_meter: {wp.sea_depth_meter}) " + f"does not match the actual sea depth at that location ({actual_depth:.1f}). " + f"The water there is deeper than planned, so deployments take longer and " + f"the ship would arrive too late at {self._describe_waypoint(wp_i, wp_next)} " + f"(projected {arrival_time.replace(microsecond=0)}, due {wp_next.time}).\n\n" + f"Please update 'sea_depth_meter' for {wp_name} to {actual_depth:.1f}, " + f"then adjust the schedule timings if needed.", + stacklevel=2, + ) + + # mark this schedule as verified (so that subsequent stages of the workflow can proceed without re-verifying) self.schedule._verified = True print("... All good to go!") @@ -129,6 +154,7 @@ def _first_overrun( self, wps_in_use: list[Port | Waypoint], ship_speed: float, + measured_depths: dict[int, float] | None = None, ) -> tuple[int, Port | Waypoint, datetime] | None: """Project the ship along the schedule and return the first waypoint it reaches late.""" time = wps_in_use[0].time @@ -139,7 +165,8 @@ def _first_overrun( for wp_i, (wp, wp_next) in enumerate( itertools.pairwise(wps_in_use), start=wps_in_use_offset ): - stationkeeping_time = self.stationkeeping_time(wp) + bathymetry = measured_depths.get(id(wp)) if measured_depths else None + stationkeeping_time = self.stationkeeping_time(wp, bathymetry) time_to_reach = _calc_sail_time( wp.location, @@ -359,8 +386,8 @@ def _check_structure(self, wps_in_use: list[Port | Waypoint]) -> None: f"Waypoint(s) {', '.join(invalid_labels)}: each waypoint should be timed after all previous waypoints", ) - def _check_in_water(self, from_data: Path | None = None) -> None: - """Check non-port waypoints sit in water rather than on land.""" + def _check_in_water(self, from_data: Path | None = None) -> dict[int, float]: + """Check non-port waypoints sit in water rather than on land, return measured seabed depth at each (keyed by waypoint id).""" try: bathymetry_field = _get_bathy_data(from_data=from_data).bathymetry except Exception as e: @@ -369,6 +396,7 @@ def _check_in_water(self, from_data: Path | None = None) -> None: ) from e land_waypoints = [] + measured_depths: dict[int, float] = {} for wp_i, wp in enumerate(self.waypoints): if isinstance(wp, Port): @@ -383,6 +411,8 @@ def _check_in_water(self, from_data: Path | None = None) -> None: ) if value == 0.0 or (isinstance(value, float) and np.isnan(value)): land_waypoints.append((public_wp, wp)) + else: + measured_depths[id(wp)] = float(np.ravel(value)[0]) except Exception as e: raise ScheduleError( f"Waypoint #{public_wp} at location {wp.location} could not be evaluated against bathymetry data. \n\n Original error: {e}" @@ -393,6 +423,8 @@ def _check_in_water(self, from_data: Path | None = None) -> None: f"The following waypoint(s) throw(s) error(s): {['#' + str(public_wp) + ' ' + str(wp) for (public_wp, wp) in land_waypoints]}\n\nINFO: They are likely on land (bathymetry data cannot be interpolated to their location(s)).\n" ) + return measured_depths + def _get_wps_in_use(self) -> list[Port | Waypoint]: """Return waypoints that are in use (i.e., have a specified time and location), i.e. excluding placeholder departure/arrival ports.""" start_slice = 0 if self.departure_port.is_in_use else 1 diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index 901c2139..74817dcb 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -214,6 +214,67 @@ def test_verify_on_land(base_expedition): ) +def _verify_against_deep_seabed(base_expedition, time_allowed_after_ctd): + """ + Verify a schedule whose CTD station is planned at 100 m but sits on a 3000 m seabed. + + At the planned depth (e.g. from MFP) the ship is held approx. 33 min; at the measured depth (e.g. from bathy fieldset; capped by the CTD's 2000 m max) approx.96 min. + The final leg to port takes approx. 36 min at 10 knots. + """ + lat = lon = np.array([0.0, 1.0, 2.0]) + ds_bathymetry = xr.Dataset( + {"deptho": (("lat", "lon"), np.full((3, 3), -3000.0))}, + coords={ + "lon": (("lon"), lon, {"units": "degrees_east"}), + "lat": (("lat"), lat, {"units": "degrees_north"}), + }, + ) + bathymetry_fieldset = parcels.FieldSet.from_sgrid_conventions( + parcels.convert.copernicusmarine_to_sgrid( + fields={"bathymetry": ds_bathymetry["deptho"]} + ) + ) + + ctd_time = datetime(2022, 1, 1, 8, 0, 0) + schedule = Schedule( + waypoints=[ + Port(location=Location(0.0, 0.0), time=datetime(2022, 1, 1, 0, 0, 0)), + Waypoint( + location=Location(0.0, 1.0), + time=ctd_time, + instrument=[InstrumentType.CTD], + sea_depth_meter=-100.0, + ), + Port(location=Location(0.0, 1.1), time=ctd_time + time_allowed_after_ctd), + ] + ) + + with patch( + "virtualship.models.expedition._get_bathy_data", + return_value=bathymetry_fieldset, + ): + _with_schedule(base_expedition, schedule).verify_schedule( + ignore_land_test=False, from_data=None + ) + + +def test_verify_warns_when_measured_bathymetry_breaks_schedule(base_expedition): + """A schedule that only fits at the planned depth warns that the real seabed breaks it.""" + with pytest.warns(UserWarning, match="actual sea depth") as record: + _verify_against_deep_seabed(base_expedition, timedelta(minutes=90)) + + # tells the user which value to put in the schedule + message = str(record[0].message) + assert "update 'sea_depth_meter' for waypoint 1 to -3000.0" in message + + +def test_verify_silent_when_measured_bathymetry_still_fits(base_expedition, recwarn): + """A deeper real seabed is not worth a warning if the schedule still has room for it.""" + _verify_against_deep_seabed(base_expedition, timedelta(hours=3)) + + assert not [w for w in recwarn if "actual sea depth" in str(w.message)] + + @pytest.mark.parametrize( "waypoints,error,match", [ From 952ce82f1424c83e65be08038650a2684ad90103 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:58:30 +0200 Subject: [PATCH 07/39] move stationkeeping_time to Expedition model; use new model of stationkeeping_time = deceleration + deployment + acceleration time across codebase --- .../expedition/simulate_schedule.py | 36 +----- .../make_realistic/problems/simulator.py | 7 +- src/virtualship/models/checkpoint.py | 8 +- src/virtualship/models/expedition.py | 112 ++++++++++-------- tests/cli/test_plan.py | 12 +- tests/expedition/test_expedition.py | 72 +++++++---- 6 files changed, 126 insertions(+), 121 deletions(-) diff --git a/src/virtualship/expedition/simulate_schedule.py b/src/virtualship/expedition/simulate_schedule.py index cbf8a430..48dca355 100644 --- a/src/virtualship/expedition/simulate_schedule.py +++ b/src/virtualship/expedition/simulate_schedule.py @@ -231,23 +231,8 @@ def _get_underway_stationary_times( ] def _get_instrument_timescosts(self, waypoint: Waypoint | Port) -> timedelta: - # port stops have no instruments; if there are no instruments, there is no time cost - if isinstance(waypoint, Port): - return timedelta() - - # if proper waypoint but there are no instruments, there is no time cost - if isinstance(waypoint, Waypoint) and waypoint.instrument is None: - return timedelta() - - # make instruments a list even if it's only a single one - instruments = ( - waypoint.instrument - if isinstance(waypoint.instrument, list) - else [waypoint.instrument] - ) - - # time costs of each measurement - time_costs = [timedelta()] + """Queue waypoint's deployments, report how long the ship is held there.""" + instruments = self._expedition._wp_instruments(waypoint) for instrument in instruments: if instrument is InstrumentType.ARGO_FLOAT: @@ -262,12 +247,6 @@ def _get_instrument_timescosts(self, waypoint: Waypoint | Port) -> timedelta: drift_days=self._expedition.instruments_config.argo_float_config.drift_days, ) ) - # TODO: would be good to avoid having to twice make sure that stationkeeping time is factored in; i.e. in schedule validity checks and here (and for CTDs and Drifters) - # TODO: makes it easy to forget to update both... - # TODO: this is likely to fall under refactoring simulate_schedule.py (i.e. #236) - time_costs.append( - self._expedition.instruments_config.argo_float_config.stationkeeping_time - ) elif instrument is InstrumentType.CTD: self._measurements_to_simulate.ctds.append( @@ -277,9 +256,6 @@ def _get_instrument_timescosts(self, waypoint: Waypoint | Port) -> timedelta: max_depth=self._expedition.instruments_config.ctd_config.max_depth_meter, ) ) - time_costs.append( - self._expedition.instruments_config.ctd_config.stationkeeping_time - ) elif instrument is InstrumentType.DRIFTER: self._measurements_to_simulate.drifters.append( @@ -289,9 +265,6 @@ def _get_instrument_timescosts(self, waypoint: Waypoint | Port) -> timedelta: lifetime=self._expedition.instruments_config.drifter_config.lifetime, ) ) - time_costs.append( - self._expedition.instruments_config.drifter_config.stationkeeping_time - ) elif instrument is InstrumentType.XBT: self._measurements_to_simulate.xbts.append( @@ -306,7 +279,4 @@ def _get_instrument_timescosts(self, waypoint: Waypoint | Port) -> timedelta: else: raise NotImplementedError("Instrument type not supported.") - # measurements are done simultaneously onboard, so return time of longest one - # TODO: docs suggest that add individual instrument stationkeeping times are cumulative, which is at odds with measurements being done simultaneously onboard here - # TODO: update one or the other? - return max(time_costs) + return self._expedition.stationkeeping_time(waypoint) diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index b02486c5..8dd98afe 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -3,7 +3,6 @@ import random import sys import time -from datetime import timedelta from pathlib import Path from typing import TYPE_CHECKING @@ -380,11 +379,7 @@ def _has_contingency(self, problem: ProblemType, problem_wp_i: int | None) -> bo self.waypoints[problem_wp_i + 1], ) - stationkeeping = ( - curr_wp.stationkeeping_time(self.expedition.instruments_config) - if not isinstance(curr_wp, Port) - else timedelta(0) - ) + stationkeeping = self.expedition.stationkeeping_time(curr_wp) sail_time = _calc_sail_time( curr_wp.location, next_wp.location, diff --git a/src/virtualship/models/checkpoint.py b/src/virtualship/models/checkpoint.py index 85625f3d..2b340b71 100644 --- a/src/virtualship/models/checkpoint.py +++ b/src/virtualship/models/checkpoint.py @@ -10,7 +10,7 @@ from virtualship.errors import CheckpointError from virtualship.instruments.types import InstrumentType -from virtualship.models.expedition import Expedition, Port, Schedule +from virtualship.models.expedition import Expedition, Schedule from virtualship.utils import ( EXPEDITION, PROJECTION, @@ -100,11 +100,7 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: if has_problem_location: problem_waypoint = new_schedule.waypoints[problem_wp_i] - stationkeeping_time = ( - problem_waypoint.stationkeeping_time(expedition.instruments_config) - if not isinstance(problem_waypoint, Port) - else timedelta(0) - ) + stationkeeping_time = expedition.stationkeeping_time(problem_waypoint) sail_time = _calc_sail_time( problem_waypoint.location, diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 8d8def8c..56667878 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -15,11 +15,11 @@ from virtualship.instruments.sensors import SENSOR_REGISTRY, SensorType, _Sensor from virtualship.instruments.types import InstrumentType from virtualship.utils import ( - INSTRUMENT_CONFIG_MAP, _calc_sail_time, _get_bathy_data, _get_public_wp, _validate_numeric_to_timedelta, + get_instrument_class, get_supported_sensors, register_instrument_config, ) @@ -140,11 +140,7 @@ def _first_overrun( for wp_i, (wp, wp_next) in enumerate( itertools.pairwise(wps_in_use), start=wps_in_use_offset ): - stationkeeping_time = ( - wp.stationkeeping_time(self.instruments_config) - if isinstance(wp, Waypoint) - else timedelta() - ) + stationkeeping_time = self.stationkeeping_time(wp) time_to_reach = _calc_sail_time( wp.location, @@ -171,6 +167,53 @@ def _describe_waypoint(self, wp_i: int, wp_next: Port | Waypoint) -> str: # +1 to get next return f"waypoint {_get_public_wp(wp_i + 1, self.schedule.waypoints)}" + def stationkeeping_time(self, waypoint: Port | Waypoint) -> timedelta: + """ + Calculate how long the ship is stationary at waypoint. + + Stationkeeping time = ship deceleration time + deployment time + ship acceleration time. + Assumes insturments are deployed concurrently, so the longest deployment time is used. + Empty waypoints (no instruments) cost nothing/ship sails through without slowing. + """ + instruments = self._wp_instruments(waypoint) + if not instruments: + return timedelta() + + deployment_time = max( + self._deployment_time(instrument, waypoint) for instrument in instruments + ) + + return ( + self.ship_config.deceleration_time + + deployment_time + + self.ship_config.acceleration_time + ) + + def _deployment_time( + self, + instrument: InstrumentType, + waypoint: Waypoint, + ) -> timedelta: + """Time an instrument holds the ship on station.""" + config = self.instruments_config.config_for(instrument) + if config is None: + return timedelta() + + return get_instrument_class(instrument).deployment_time( + config, waypoint.sea_depth_meter + ) + + @staticmethod + def _wp_instruments(waypoint: Port | Waypoint) -> list[InstrumentType]: + """Instruments deployed at a waypoint.""" + if isinstance(waypoint, Port) or not waypoint.instrument: + return [] + return ( + waypoint.instrument + if isinstance(waypoint.instrument, list) + else [waypoint.instrument] + ) + def _annotate(self): """ Add port/waypoint comments/annotations to the expedition.yaml file. @@ -391,39 +434,6 @@ def serialize_instrument(self, instrument): return [inst.value for inst in instrument] return instrument.value if instrument else None - def stationkeeping_time( - self, - instruments_config: InstrumentsConfig, - instrument_config_map: dict = INSTRUMENT_CONFIG_MAP, - ) -> timedelta: - """Calculate how much time is required to carry out all instrument deployments at this waypoint.""" - wp_instruments = ( - self.instrument - if isinstance(self.instrument, list) - else ([self.instrument] if self.instrument else []) # safe handling of None - ) - - valid_instrument_configs = [ - iconfig for _, iconfig in instruments_config.__dict__.items() if iconfig - ] - - wp_instrument_configs = [] - for iconfig in valid_instrument_configs: - for instr in wp_instruments: - if ( - instrument_config_map.get(instr) == iconfig.__class__.__name__ - and iconfig - not in wp_instrument_configs # avoid duplicates (would happen when multiple drifter deployments at same wp) - ): - wp_instrument_configs.append(iconfig) - - cumulative_stationkeeping_time = timedelta() - for iconfig in wp_instrument_configs: - if hasattr(iconfig, "stationkeeping_time"): - cumulative_stationkeeping_time += iconfig.stationkeeping_time - - return cumulative_stationkeeping_time - ## @@ -710,6 +720,21 @@ class InstrumentsConfig(pydantic.BaseModel): model_config = pydantic.ConfigDict(extra="forbid") + _CONFIG_FIELDS: ClassVar[dict[InstrumentType, str]] = { + InstrumentType.ARGO_FLOAT: "argo_float_config", + InstrumentType.DRIFTER: "drifter_config", + InstrumentType.XBT: "xbt_config", + InstrumentType.CTD: "ctd_config", + InstrumentType.ADCP: "adcp_config", + InstrumentType.UNDERWATER_ST: "ship_underwater_st_config", + } + + def config_for( + self, instrument_type: InstrumentType + ) -> _InstrumentConfigMixin | None: + """The configuration for an instrument type, or None if it is not configured.""" + return getattr(self, self._CONFIG_FIELDS[instrument_type]) + def verify(self, instruments_in_expedition: Collection[InstrumentType]) -> None: """ Verify instrument configurations against the instruments actually being deployed. @@ -717,14 +742,7 @@ def verify(self, instruments_in_expedition: Collection[InstrumentType]) -> None: Removes instrument configs not present in the schedule and checks that all scheduled instruments are configured. Raises ConfigError if any scheduled instrument is missing a config. """ - instrument_config_map = { - InstrumentType.ARGO_FLOAT: "argo_float_config", - InstrumentType.DRIFTER: "drifter_config", - InstrumentType.XBT: "xbt_config", - InstrumentType.CTD: "ctd_config", - InstrumentType.ADCP: "adcp_config", - InstrumentType.UNDERWATER_ST: "ship_underwater_st_config", - } + instrument_config_map = self._CONFIG_FIELDS # Remove configs for unused instruments for inst_type, config_attr in instrument_config_map.items(): if ( diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index d799fdc0..96d23527 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -130,12 +130,12 @@ async def test_UI_changes(tmp_path): ), Waypoint( location=Location(0.01, 0.01), - time=datetime(2022, 1, 1, 1, 0, 0), + time=datetime(2022, 1, 1, 3, 0, 0), instrument=["CTD"], ), Waypoint( location=Location(0.02, 0.02), - time=datetime(2022, 1, 1, 2, 0, 0), + time=datetime(2022, 1, 1, 6, 0, 0), instrument=["CTD"], ), Port( @@ -274,7 +274,7 @@ async def test_sensor_toggle_saved_to_yaml(tmp_path): ), Waypoint( location=Location(0.01, 0.01), - time=datetime(2022, 1, 1, 1, 0, 0), + time=datetime(2022, 1, 1, 3, 0, 0), instrument=None, ), Port(location=None, time=None), @@ -325,7 +325,7 @@ async def test_deselecting_all_sensors_on_active_instrument_blocks_save(tmp_path ), Waypoint( location=Location(0.01, 0.01), - time=datetime(2022, 1, 1, 1, 0, 0), + time=datetime(2022, 1, 1, 3, 0, 0), instrument=None, ), Port(location=None, time=None), @@ -370,7 +370,7 @@ async def test_deselecting_all_sensors_on_inactive_instrument(tmp_path): ), Waypoint( location=Location(0.01, 0.01), - time=datetime(2022, 1, 1, 1, 0, 0), + time=datetime(2022, 1, 1, 3, 0, 0), instrument=None, ), Port(location=None, time=None), @@ -428,7 +428,7 @@ async def test_sensor_initial_state_reflects_config(tmp_path): ), Waypoint( location=Location(0.01, 0.01), - time=datetime(2022, 1, 1, 1, 0, 0), + time=datetime(2022, 1, 1, 3, 0, 0), instrument=None, ), Port(location=None, time=None), diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index a7465cbb..d44e8cd5 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -82,51 +82,77 @@ def test_get_instruments(base_expedition) -> None: } -def test_waypoint_stationkeeping_time(expedition) -> None: - """Waypoint.stationkeeping_time sums the stationkeeping time of each unique instrument config present and not duplicating repeated instruments (e.g. multiple drifters).""" - instruments_config = expedition.instruments_config +def test_stationkeeping_time_uses_slowest_deployment(expedition) -> None: + """Deployments run concurrently while the ship is stopped, so the slowest one governs.""" + ship_config = expedition.ship_config + ctd_config = expedition.instruments_config.ctd_config waypoint = Waypoint( location=Location(0, 0), + sea_depth_meter=-500.0, instrument=[ InstrumentType.CTD, InstrumentType.ARGO_FLOAT, - InstrumentType.XBT, # has no stationkeeping time, should be ignored + InstrumentType.XBT, InstrumentType.DRIFTER, InstrumentType.DRIFTER, # two drifter deployments ], ) - expected = ( - instruments_config.ctd_config.stationkeeping_time - + instruments_config.argo_float_config.stationkeeping_time - + instruments_config.drifter_config.stationkeeping_time + # only the CTD holds the ship; it is winched to the seabed at 500 m and hauled back + cast = timedelta( + seconds=2 + * abs(-500.0 - ctd_config.min_depth_meter) + / ctd_config.winch_speed_meter_per_second ) - assert waypoint.stationkeeping_time(instruments_config) == expected + expected = ship_config.deceleration_time + cast + ship_config.acceleration_time + assert expedition.stationkeeping_time(waypoint) == expected -def test_waypoint_stationkeeping_time_single_instrument(expedition) -> None: - """A bare (non-list) InstrumentType value on a waypoint is handled the same as a single element list.""" - instruments_config = expedition.instruments_config - waypoint = Waypoint(location=Location(0, 0), instrument=InstrumentType.CTD) +def test_stationkeeping_time_capped_by_sea_depth(expedition) -> None: + """A CTD cannot be winched below the seabed, so a shallow waypoint is quicker.""" + deep = Waypoint( + location=Location(0, 0), instrument=InstrumentType.CTD, sea_depth_meter=-3000.0 + ) + shallow = Waypoint( + location=Location(0, 0), instrument=InstrumentType.CTD, sea_depth_meter=-100.0 + ) + + # deep waypoint is limited by the CTD's own configured maximum, not the seabed + unbounded = Waypoint(location=Location(0, 0), instrument=InstrumentType.CTD) - assert ( - waypoint.stationkeeping_time(instruments_config) - == instruments_config.ctd_config.stationkeeping_time + assert expedition.stationkeeping_time(shallow) < expedition.stationkeeping_time( + deep ) + assert expedition.stationkeeping_time(deep) == expedition.stationkeeping_time( + unbounded + ) + +def test_stationkeeping_time_single_instrument(expedition) -> None: + """A bare (non-list) InstrumentType value is handled the same as a single element list.""" + bare = Waypoint( + location=Location(0, 0), instrument=InstrumentType.CTD, sea_depth_meter=-500.0 + ) + listed = Waypoint( + location=Location(0, 0), instrument=[InstrumentType.CTD], sea_depth_meter=-500.0 + ) + + assert expedition.stationkeeping_time(bare) == expedition.stationkeeping_time( + listed + ) -def test_waypoint_stationkeeping_time_no_instruments(expedition) -> None: - """Waypoints with no instruments (None or empty list) have zero stationkeeping time.""" - instruments_config = expedition.instruments_config - assert Waypoint(location=Location(0, 0), instrument=None).stationkeeping_time( - instruments_config +def test_stationkeeping_time_no_instruments(expedition) -> None: + """The ship doesn't slows down where nothing is deployed, therefore ports and bare waypoints cost nothing.""" + assert expedition.stationkeeping_time( + Waypoint(location=Location(0, 0), instrument=None) ) == timedelta(0) - assert Waypoint(location=Location(0, 0), instrument=[]).stationkeeping_time( - instruments_config + assert expedition.stationkeeping_time( + Waypoint(location=Location(0, 0), instrument=[]) ) == timedelta(0) + assert expedition.stationkeeping_time(Port(location=Location(0, 0))) == timedelta(0) def test_verify_on_land(base_expedition): From 71eb5915d59213a946edad57bb8bc031f14f1745 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:26:05 +0200 Subject: [PATCH 08/39] use MFP export sea depth in expedition.yaml on `virtualship init` --- src/virtualship/cli/_initialise.py | 12 ++++++++- tests/cli/test_initialise.py | 39 ++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/virtualship/cli/_initialise.py b/src/virtualship/cli/_initialise.py index 64548b19..44272dbc 100644 --- a/src/virtualship/cli/_initialise.py +++ b/src/virtualship/cli/_initialise.py @@ -102,7 +102,17 @@ def _mfp_to_yaml(file_path: Path, start_date: str, output_path: Path): has_latlon = lat is not None and lon is not None waypoints.append(Port(location=loc, time=time_val if has_latlon else None)) else: - waypoints.append(Waypoint(instrument=None, location=loc, time=time_val)) + sea_depth = ( + None if pd.isna(row["Sea Depth"]) else -abs(float(row["Sea Depth"])) + ) # MFP reports sea depth as positive, but here we use negative elevations + waypoints.append( + Waypoint( + instrument=None, + location=loc, + time=time_val, + sea_depth_meter=sea_depth, + ) + ) previous_timedelta = ( row["Total Time"] if pd.notna(row["Total Time"]) else timedelta(0) diff --git a/tests/cli/test_initialise.py b/tests/cli/test_initialise.py index 191d87a4..50273c33 100644 --- a/tests/cli/test_initialise.py +++ b/tests/cli/test_initialise.py @@ -163,6 +163,45 @@ def test_mfp_to_yaml_success(request, fixture_name, tmp_path): assert isinstance(data.schedule.waypoints[1], Waypoint) +def test_mfp_sea_depth_is_positive_and_negated_on_ingest( + valid_excel_mfp_file, tmp_path +): + """Ingest positive MFP sea depths and negate in VirtualShip internal representation.""" + # MFP export gives depths as positive metres below the surface + source_depths = valid_mfp_data()["Sea Depth"] + assert (source_depths > 0).all(), ( + "MFP appears to not be reporting sea depths as positive values, which is a change in behaviour relevant to the VirtualShip workflow (expects positive which are then negated in VirtualShip). Please check the MFP export." + ) + + yaml_output_path = tmp_path / "expedition.yaml" + _mfp_to_yaml(valid_excel_mfp_file, "2023-10-20 01:00:00", yaml_output_path) + + expedition = Expedition.from_yaml(yaml_output_path) + waypoints = [wp for wp in expedition.schedule.waypoints if isinstance(wp, Waypoint)] + + # sign flipped on the way in, magnitude preserved, and the order is kept + station_depths = source_depths[1:-1].tolist() # excluding the two port rows + assert [wp.sea_depth_meter for wp in waypoints] == [-d for d in station_depths] + assert all(wp.sea_depth_meter < 0 for wp in waypoints) + + +def test_mfp_sea_depth_absent_leaves_waypoint_depth_unset(tmp_path): + """A blank sea depth is left as None, and not read as e.g. a depth of zero.""" + df = valid_mfp_data() + df["Sea Depth"] = [None] * len(df) + mfp_file = tmp_path / "no_depths.xlsx" + df.to_excel(mfp_file, index=False) + + yaml_output_path = tmp_path / "expedition.yaml" + _mfp_to_yaml(mfp_file, "2023-10-20 01:00:00", yaml_output_path) + + expedition = Expedition.from_yaml(yaml_output_path) + waypoints = [wp for wp in expedition.schedule.waypoints if isinstance(wp, Waypoint)] + + assert waypoints, "expected the fixture to produce waypoints, but got none" + assert all(wp.sea_depth_meter is None for wp in waypoints) + + @pytest.mark.parametrize( "fixture_name,error,match", [ From 45b9ed510d208115f49e466ce544d3a529f3c282 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:59:16 +0200 Subject: [PATCH 09/39] remove now redundant INSTRUMENT_CONFIG_MAP --- src/virtualship/models/expedition.py | 7 ------- src/virtualship/utils.py | 12 ------------ 2 files changed, 19 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 56667878..82efea81 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -21,7 +21,6 @@ _validate_numeric_to_timedelta, get_instrument_class, get_supported_sensors, - register_instrument_config, ) from .location import Location @@ -480,7 +479,6 @@ def _validate_lifetime(cls, value: int | float | timedelta) -> timedelta: return _validate_numeric_to_timedelta(value, "days") -@register_instrument_config(InstrumentType.ARGO_FLOAT) class ArgoFloatConfig(_InstrumentConfigMixin, pydantic.BaseModel): """Configuration for argos floats.""" @@ -527,7 +525,6 @@ def _drift_days_less_than_cycle_days(self) -> ArgoFloatConfig: return self -@register_instrument_config(InstrumentType.ADCP) class ADCPConfig(_InstrumentConfigMixin, pydantic.BaseModel): """Configuration for ADCP instrument.""" @@ -566,7 +563,6 @@ def active_variables(self) -> dict[str, str]: return variables -@register_instrument_config(InstrumentType.CTD) class CTDConfig(_InstrumentConfigMixin, pydantic.BaseModel): """Configuration for CTD instrument.""" @@ -602,7 +598,6 @@ class CTDConfig(_InstrumentConfigMixin, pydantic.BaseModel): model_config = pydantic.ConfigDict(populate_by_name=True) -@register_instrument_config(InstrumentType.UNDERWATER_ST) class ShipUnderwaterSTConfig(_InstrumentConfigMixin, pydantic.BaseModel): """Configuration for underwater ST.""" @@ -628,7 +623,6 @@ class ShipUnderwaterSTConfig(_InstrumentConfigMixin, pydantic.BaseModel): model_config = pydantic.ConfigDict(populate_by_name=True) -@register_instrument_config(InstrumentType.DRIFTER) class DrifterConfig(_InstrumentConfigMixin, pydantic.BaseModel): """Configuration for drifters.""" @@ -655,7 +649,6 @@ class DrifterConfig(_InstrumentConfigMixin, pydantic.BaseModel): model_config = pydantic.ConfigDict(populate_by_name=True) -@register_instrument_config(InstrumentType.XBT) class XBTConfig(_InstrumentConfigMixin, pydantic.BaseModel): """Configuration for xbt instrument.""" diff --git a/src/virtualship/utils.py b/src/virtualship/utils.py index c6dd4762..12c41204 100644 --- a/src/virtualship/utils.py +++ b/src/virtualship/utils.py @@ -163,18 +163,6 @@ def get_supported_sensors(instrument_type): return supported -# map for instrument type to instrument config (pydantic basemodel) names -INSTRUMENT_CONFIG_MAP = {} - - -def register_instrument_config(instrument_type): - def decorator(cls): - INSTRUMENT_CONFIG_MAP[instrument_type] = cls.__name__ - return cls - - return decorator - - # ===================================================== # SECTION: helper functions # ===================================================== From 280970ff0c031b9760b068be0197c08ec612e688 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:27:06 +0200 Subject: [PATCH 10/39] add ship deceleration and acceleration times in the plan tool --- src/virtualship/cli/_plan.py | 40 +++++++++++++++++++++- tests/cli/test_plan.py | 64 +++++++++++++++++++++++++++++++++++- 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index c289e02a..2122fdec 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -352,6 +352,30 @@ def compose(self) -> ComposeResult: ) yield Label("", id="validation-failure-label-speed", classes="-hidden") + with Horizontal(classes="ship_speed"): + for attr, label, input_id in ( + ("deceleration_time", "Decelerate (mins):", "deceleration"), + ("acceleration_time", "Accelerate (mins):", "acceleration"), + ): + current = getattr(self.expedition.ship_config, attr, None) + yield Label(f"[b]{label}[/b]") + yield Input( + id=input_id, + type=type_to_textual(get_field_type(ShipConfig, attr)), + validators=[ + Function( + validator, + f"INVALID: value must be {validator.__doc__.lower()}", + ) + for validator in group_validators(ShipConfig, attr) + ], + classes="ship_speed_input", + placeholder="minutes", + value=str(current.total_seconds() / 60.0) + if current + else "", + ) + with Horizontal(classes="ts-section"): yield Label("[b]Onboard Temperature/Salinity:[/b]") yield Switch( @@ -541,10 +565,24 @@ def _update_ship_speed(self): attr = "ship_speed_knots" field_type = get_field_type(type(self.expedition.ship_config), attr) value = field_type(self.query_one("#speed").value) + + # entered in minutes, stored as timedeltas + times = { + attr_name: datetime.timedelta( + minutes=float(self.query_one(f"#{input_id}").value) + ) + for attr_name, input_id in ( + ("deceleration_time", "deceleration"), + ("acceleration_time", "acceleration"), + ) + } + ShipConfig.model_validate( - {**self.expedition.ship_config.model_dump(), attr: value} + {**self.expedition.ship_config.model_dump(), attr: value, **times} ) self.expedition.ship_config.ship_speed_knots = value + for attr_name, time_value in times.items(): + setattr(self.expedition.ship_config, attr_name, time_value) def _update_instrument_configs(self): instruments_config = self.expedition.instruments_config diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 820b73b0..64fca32a 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -215,7 +215,11 @@ async def test_UI_changes(tmp_path): assert saved_wps[2].location.lon == float(NEW_LON) assert saved_wps[1].instrument == [InstrumentType.XBT] - # check schedule.verify() methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) + # the ship's slow-down / speed-up times survive a round trip through the UI + assert saved_expedition["ship_config"]["deceleration_time_minutes"] == 15.0 + assert saved_expedition["ship_config"]["acceleration_time_minutes"] == 15.0 + + # check expedition.verify_schedule() methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) invalid_speed = "0.0001" await simulate_input(pilot, ship_speed_input, invalid_speed) await pilot.click(save_button) @@ -225,6 +229,64 @@ async def test_UI_changes(tmp_path): assert "*** Error saving changes ***" in args[0] +@pytest.mark.asyncio +async def test_ship_decel_accel_editable_and_saved(tmp_path): + """The ship's deceleration and acceleration times can be edited in the UI and are written back.""" + _make_expedition( + tmp_path, + [ + Port(location=None, time=None), + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 0, 0, 0), + instrument=["CTD"], + ), + Waypoint( + location=Location(0.01, 0.01), + time=datetime(2022, 1, 1, 6, 0, 0), + instrument=None, + ), + Port(location=None, time=None), + ], + ) + + app = PlanApp(path=tmp_path) + async with app.run_test(size=(160, 140)) as pilot: + await pilot.pause(0.5) + plan_screen = pilot.app.screen + plan_screen.notify = MagicMock() + expedition_editor = plan_screen.query_one(ExpeditionEditor) + + speed_collapsible = expedition_editor.query_one( + "#speed_collapsible", Collapsible + ) + speed_collapsible.collapsed = False + await pilot.pause() + + await simulate_input( + pilot, expedition_editor.query_one("#deceleration", Input), "20" + ) + await simulate_input( + pilot, expedition_editor.query_one("#acceleration", Input), "25" + ) + + await pilot.click(plan_screen.query_one("#save_button", Button)) + await pilot.pause(0.5) + + calls = plan_screen.notify.call_args_list + assert any( + call[0][0] == "Changes saved successfully" + and call[1].get("severity") == "information" + for call in calls + ) + + with open(tmp_path / EXPEDITION) as f: + saved = yaml.safe_load(f) + + assert saved["ship_config"]["deceleration_time_minutes"] == 20.0 + assert saved["ship_config"]["acceleration_time_minutes"] == 25.0 + + @pytest.mark.asyncio async def test_UI_opens_with_null_time_and_instrument(tmp_path): """Test that the UI opens correctly when waypoints have time: null and instrument: null.""" From d1aad15892af847c4613210ee75dc394020aee5e Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:00:07 +0200 Subject: [PATCH 11/39] remove stationkeeping_time fields from instrument configs --- src/virtualship/cli/_plan.py | 3 -- src/virtualship/models/expedition.py | 35 +++++-------------- src/virtualship/static/expedition.yaml | 3 -- tests/cli/test_plan.py | 1 - .../expedition/expedition_dir/expedition.yaml | 3 -- tests/expedition/test_simulate_schedule.py | 7 ---- tests/instruments/test_argo_float.py | 3 -- tests/instruments/test_ctd.py | 7 ---- tests/instruments/test_drifter.py | 4 --- 9 files changed, 9 insertions(+), 57 deletions(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index fbe9b194..c289e02a 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -233,7 +233,6 @@ def _raise_logged_unexpected(e: Exception, path, context_message: str): {"name": "max_depth_meter"}, {"name": "min_depth_meter"}, {"name": "winch_speed_meter_per_second"}, - {"name": "stationkeeping_time", "minutes": True}, ], }, "xbt_config": { @@ -258,7 +257,6 @@ def _raise_logged_unexpected(e: Exception, path, context_message: str): {"name": "vertical_speed_meter_per_second"}, {"name": "cycle_days"}, {"name": "drift_days"}, - {"name": "stationkeeping_time", "minutes": True}, {"name": "lifetime", "days": True}, ], }, @@ -269,7 +267,6 @@ def _raise_logged_unexpected(e: Exception, path, context_message: str): "attributes": [ {"name": "depth_meter"}, {"name": "lifetime", "days": True}, - {"name": "stationkeeping_time", "minutes": True}, ], }, } diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 82efea81..1bd6b21d 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -458,13 +458,11 @@ def active_variables(self) -> dict[str, str]: """FieldSet-key → Copernicus-variable mapping for enabled sensors.""" return SensorConfig.build_variables(self.sensors) - @pydantic.field_serializer("stationkeeping_time", "period", check_fields=False) + @pydantic.field_serializer("period", check_fields=False) def _serialize_minutes(self, value: timedelta, _info) -> float: return value.total_seconds() / 60.0 - @pydantic.field_validator( - "stationkeeping_time", "period", mode="before", check_fields=False - ) + @pydantic.field_validator("period", mode="before", check_fields=False) @classmethod def _validate_minutes(cls, value: int | float | timedelta) -> timedelta: return _validate_numeric_to_timedelta(value, "minutes") @@ -497,12 +495,6 @@ class ArgoFloatConfig(_InstrumentConfigMixin, pydantic.BaseModel): gt=timedelta(), ) - stationkeeping_time: timedelta = pydantic.Field( - serialization_alias="stationkeeping_time_minutes", - validation_alias="stationkeeping_time_minutes", - gt=timedelta(), - ) - sensors: list[SensorConfig] = pydantic.Field( default_factory=lambda: [ SensorConfig(sensor_type=SensorType.TEMPERATURE), @@ -513,7 +505,7 @@ class ArgoFloatConfig(_InstrumentConfigMixin, pydantic.BaseModel): ), ) - model_config = pydantic.ConfigDict(populate_by_name=True) + model_config = pydantic.ConfigDict(populate_by_name=True, extra="forbid") @pydantic.model_validator(mode="after") def _drift_days_less_than_cycle_days(self) -> ArgoFloatConfig: @@ -547,7 +539,7 @@ class ADCPConfig(_InstrumentConfigMixin, pydantic.BaseModel): ), ) - model_config = pydantic.ConfigDict(populate_by_name=True) + model_config = pydantic.ConfigDict(populate_by_name=True, extra="forbid") def active_variables(self) -> dict[str, str]: """ @@ -569,11 +561,6 @@ class CTDConfig(_InstrumentConfigMixin, pydantic.BaseModel): _instrument_type: ClassVar[InstrumentType] = InstrumentType.CTD _instrument_name: ClassVar[str] = "CTD" - stationkeeping_time: timedelta = pydantic.Field( - serialization_alias="stationkeeping_time_minutes", - validation_alias="stationkeeping_time_minutes", - gt=timedelta(), - ) min_depth_meter: float = pydantic.Field(le=0.0) max_depth_meter: float = pydantic.Field(le=0.0) winch_speed_meter_per_second: float = pydantic.Field(gt=0.0) @@ -595,7 +582,7 @@ class CTDConfig(_InstrumentConfigMixin, pydantic.BaseModel): ), ) - model_config = pydantic.ConfigDict(populate_by_name=True) + model_config = pydantic.ConfigDict(populate_by_name=True, extra="forbid") class ShipUnderwaterSTConfig(_InstrumentConfigMixin, pydantic.BaseModel): @@ -620,7 +607,7 @@ class ShipUnderwaterSTConfig(_InstrumentConfigMixin, pydantic.BaseModel): ), ) - model_config = pydantic.ConfigDict(populate_by_name=True) + model_config = pydantic.ConfigDict(populate_by_name=True, extra="forbid") class DrifterConfig(_InstrumentConfigMixin, pydantic.BaseModel): @@ -635,18 +622,12 @@ class DrifterConfig(_InstrumentConfigMixin, pydantic.BaseModel): validation_alias="lifetime_days", gt=timedelta(), ) - stationkeeping_time: timedelta = pydantic.Field( - serialization_alias="stationkeeping_time_minutes", - validation_alias="stationkeeping_time_minutes", - gt=timedelta(), - ) - sensors: list[SensorConfig] = pydantic.Field( default_factory=lambda: [SensorConfig(sensor_type=SensorType.TEMPERATURE)], description=("Sensors fitted to the drifter. Supported: TEMPERATURE. "), ) - model_config = pydantic.ConfigDict(populate_by_name=True) + model_config = pydantic.ConfigDict(populate_by_name=True, extra="forbid") class XBTConfig(_InstrumentConfigMixin, pydantic.BaseModel): @@ -665,6 +646,8 @@ class XBTConfig(_InstrumentConfigMixin, pydantic.BaseModel): description=("Sensors fitted to the XBT. Supported: TEMPERATURE. "), ) + model_config = pydantic.ConfigDict(populate_by_name=True, extra="forbid") + class InstrumentsConfig(pydantic.BaseModel): """Configuration of instruments.""" diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index 2d62bf81..b8cd907e 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -14,7 +14,6 @@ instruments_config: max_depth_meter: -2000.0 min_depth_meter: 0.0 vertical_speed_meter_per_second: -0.1 - stationkeeping_time_minutes: 20.0 lifetime_days: 63.0 sensors: - TEMPERATURE @@ -23,7 +22,6 @@ instruments_config: max_depth_meter: -2000.0 min_depth_meter: -11.0 winch_speed_meter_per_second: 1.0 - stationkeeping_time_minutes: 50.0 sensors: - TEMPERATURE - SALINITY @@ -37,7 +35,6 @@ instruments_config: drifter_config: depth_meter: -1.0 lifetime_days: 42.0 - stationkeeping_time_minutes: 20.0 sensors: - TEMPERATURE xbt_config: diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 96d23527..cea9f4f1 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -407,7 +407,6 @@ async def test_deselecting_all_sensors_on_inactive_instrument(tmp_path): async def test_sensor_initial_state_reflects_config(tmp_path): """Sensor switches should reflect the enabled sensors already stored in the config.""" ctd_config = CTDConfig( - stationkeeping_time_minutes=50, winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, diff --git a/tests/expedition/expedition_dir/expedition.yaml b/tests/expedition/expedition_dir/expedition.yaml index 20030d4f..4141e899 100644 --- a/tests/expedition/expedition_dir/expedition.yaml +++ b/tests/expedition/expedition_dir/expedition.yaml @@ -39,17 +39,14 @@ instruments_config: max_depth_meter: -2000.0 min_depth_meter: 0.0 vertical_speed_meter_per_second: -0.1 - stationkeeping_time_minutes: 20.0 lifetime_days: 63.0 ctd_config: max_depth_meter: -2000.0 min_depth_meter: -11.0 winch_speed_meter_per_second: 1.0 - stationkeeping_time_minutes: 50.0 drifter_config: depth_meter: -1.0 lifetime_days: 28.0 - stationkeeping_time_minutes: 20.0 ship_underwater_st_config: period_minutes: 5.0 ship_config: diff --git a/tests/expedition/test_simulate_schedule.py b/tests/expedition/test_simulate_schedule.py index 5e7400df..19d50fec 100644 --- a/tests/expedition/test_simulate_schedule.py +++ b/tests/expedition/test_simulate_schedule.py @@ -41,13 +41,6 @@ def test_time_in_minutes_in_ship_schedule() -> None: """Test whether the pydantic serializer picks up the time *in minutes* in the ship schedule.""" instruments_config = Expedition.from_yaml(TEST_EXPEDITION_YAML).instruments_config assert instruments_config.adcp_config.period == timedelta(minutes=5) - assert instruments_config.ctd_config.stationkeeping_time == timedelta(minutes=50) - assert instruments_config.argo_float_config.stationkeeping_time == timedelta( - minutes=20 - ) - assert instruments_config.drifter_config.stationkeeping_time == timedelta( - minutes=20 - ) assert instruments_config.ship_underwater_st_config.period == timedelta(minutes=5) ship_config = Expedition.from_yaml(TEST_EXPEDITION_YAML).ship_config diff --git a/tests/instruments/test_argo_float.py b/tests/instruments/test_argo_float.py index b52ae766..5ca001f8 100644 --- a/tests/instruments/test_argo_float.py +++ b/tests/instruments/test_argo_float.py @@ -36,7 +36,6 @@ VERTICAL_SPEED = -0.10 CYCLE_DAYS = 10 DRIFT_DAYS = 9 -STATIONKEEPING_TIME = 10 @pytest.fixture @@ -49,7 +48,6 @@ def argo_config_kwargs(): "vertical_speed_meter_per_second": VERTICAL_SPEED, "cycle_days": CYCLE_DAYS, "drift_days": DRIFT_DAYS, - "stationkeeping_time_minutes": STATIONKEEPING_TIME, } @@ -162,7 +160,6 @@ class DummyExpedition: cycle_days=CYCLE_DAYS, drift_days=DRIFT_DAYS, lifetime=lifetime, - stationkeeping_time_minutes=STATIONKEEPING_TIME, sensors=sensors, ) ) diff --git a/tests/instruments/test_ctd.py b/tests/instruments/test_ctd.py index f2c60344..5aeca9b0 100644 --- a/tests/instruments/test_ctd.py +++ b/tests/instruments/test_ctd.py @@ -28,7 +28,6 @@ BASE_TIME = datetime.datetime.strptime("1950-01-01", "%Y-%m-%d") MIN_DEPTH = -11 MAX_DEPTH = -2000 -STATIONKEEPING_TIME = 50 def create_dummy_expedition( @@ -53,7 +52,6 @@ class DummyExpedition: instruments_config = InstrumentsConfig( ctd_config=CTDConfig( - stationkeeping_time_minutes=STATIONKEEPING_TIME, winch_speed_meter_per_second=1.0, min_depth_meter=MIN_DEPTH, max_depth_meter=MAX_DEPTH, @@ -329,7 +327,6 @@ def test_simulate_ctds_multiple_waypoints(tmpdir) -> None: def test_ctd_sensor_config_active_variables() -> None: """active_variables() only returns variables for enabled sensors.""" config_both = CTDConfig( - stationkeeping_time_minutes=50, winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, @@ -341,7 +338,6 @@ def test_ctd_sensor_config_active_variables() -> None: assert config_both.active_variables() == {"T": "thetao", "o2": "o2"} config_temp_only = CTDConfig( - stationkeeping_time_minutes=50, winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, @@ -355,7 +351,6 @@ def test_ctd_sensor_config_active_variables() -> None: def test_ctd_sensor_config_yaml() -> None: """CTDConfig sensors survive YAML serialisation.""" config = CTDConfig( - stationkeeping_time_minutes=50, winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, @@ -440,7 +435,6 @@ def test_ctd_supported_sensors(): def test_ctd_config_default_sensors(): """CTDConfig defaults to all supported sensors (phys + bgc).""" config = CTDConfig( - stationkeeping_time_minutes=50, winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, @@ -464,7 +458,6 @@ def test_ctd_config_unsupported_sensor_rejected(): """Unsupported sensor on CTD is rejected.""" with pytest.raises(pydantic.ValidationError, match="does not support"): CTDConfig( - stationkeeping_time_minutes=50, winch_speed_meter_per_second=1.0, min_depth_meter=-11.0, max_depth_meter=-2000.0, diff --git a/tests/instruments/test_drifter.py b/tests/instruments/test_drifter.py index 211a7f55..86e096b8 100644 --- a/tests/instruments/test_drifter.py +++ b/tests/instruments/test_drifter.py @@ -54,7 +54,6 @@ class DummyExpedition: drifter_config=DrifterConfig( lifetime=lifetime, depth_meter=depth, - stationkeeping_time_minutes=10, sensors=sensors, ) ) @@ -344,7 +343,6 @@ def test_drifter_disabled_sensor_absent_from_output(tmpdir) -> None: DrifterConfig( lifetime=LIFETIME, depth_meter=DEPLOY_DEPTH, - stationkeeping_time_minutes=10, sensors=[], ) @@ -354,7 +352,6 @@ def test_drifter_config_default_sensors(): config = DrifterConfig( lifetime=LIFETIME, depth_meter=DEPLOY_DEPTH, - stationkeeping_time_minutes=10, ) assert config.sensors[0].sensor_type is SensorType.TEMPERATURE @@ -365,7 +362,6 @@ def test_drifter_config_unsupported_sensor_rejected(): DrifterConfig( lifetime=LIFETIME, depth_meter=DEPLOY_DEPTH, - stationkeeping_time_minutes=10, sensors=[SensorConfig(sensor_type=SensorType.VELOCITY)], ) From 1405d71cbd98d46a474a75a2291456bdb86dfcae Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:39:11 +0200 Subject: [PATCH 12/39] update docs to reflect changes --- .../tutorials/working_with_expedition_yaml.md | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index d7a492e7..a827c517 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -117,7 +117,15 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint. This value matters because it determines how a long a deployment takes if there is a depth dependence, e.g. for a CTD that has to be lowered and raised. If you leave it as `null`, VirtualShip doesn't know how deep the water is and assumes the CTD descends all the way to its configured `max_depth_meter`. That is the slowest possible cast, so your schedule is verified against a worst case rather than an optimistic one in the simulations. +- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint, as a negative number of metres (e.g. `-1500.0` for a site with a depth of 1500 m). + +```{tip} +Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. Note this is the opposite sign convention to the MFP export, which reports sea depth as a positive number. +``` + +##### Comments + +You may notice comments such as `# Port of Departure`, `# Waypoint 1`, `# Waypoint 2`, etc. next to each entry in the `waypoints` list, as in the example above. These are added automatically whenever `virtualship init` or `virtualship plan` writes the file, purely to aid readability. They are not required when editing the file by hand and won't be automatically added/renumbered when doing so. #### 2. `instruments_config` @@ -161,17 +169,3 @@ This section contains setting related to the ship itself, specifically: - **Ship speed (`ship_speed_knots`)**: The speed of the ship in knots (nautical miles per hour; where 1 knot = 1.852 km/h). Note in most cases this should be left as the default value unless there is a specific reason to change it. - **Deceleration and acceleration times (`deceleration_time_minutes`, `acceleration_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. - -### How long the ship spends at each waypoint - -When VirtualShip checks whether your schedule is achievable, it needs to know how long the ship is held stationary at each waypoint. That time is made up of three parts: - -```text -time at waypoint = deceleration + slowest deployment + acceleration -``` - -A few consequences are worth understanding, because they determine whether your schedule passes verification: - -- **Waypoints with no instruments cost nothing.** If `instrument` is `null` or an empty list, the ship does not slow down at all and simply sails past. - -- **Deployments happen side by side, so only the slowest one counts.** Different teams onboard work in parallel, so the total time is taken as whichever deployment takes longest. From 9e0b3ffea4e6951bc375c7a28059a70eab9c091e Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:37:54 +0200 Subject: [PATCH 13/39] update docs to reflect new stationkeeping_time model + information on what MFP assumes --- docs/user-guide/assignments/sail_the_ship.md | 16 +++++++++++- docs/user-guide/quickstart.md | 6 +++++ .../tutorials/working_with_expedition_yaml.md | 26 +++++++++++++++---- src/virtualship/static/expedition.yaml | 5 ++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/docs/user-guide/assignments/sail_the_ship.md b/docs/user-guide/assignments/sail_the_ship.md index bd60aa67..367afce6 100644 --- a/docs/user-guide/assignments/sail_the_ship.md +++ b/docs/user-guide/assignments/sail_the_ship.md @@ -92,6 +92,12 @@ The `{CruiseDataExport}.xlsx` in the command above refers to the `.xlsx` file ex The `--start-date` should be set to your intended departure date/time. MFP's estimated travel and station durations are then used to automatically fill in a time for each waypoint relative to this start date. ``` +```{important} +MFP usually allows a default of **45 minutes** stationkeeping time at each station. This may be enough for your expedition, but it may not be, so expect to adjust some of your timings once you have chosen your instruments (in step 5) [below](#waypoint-datetimes)). + +Hint: pay particular attention to CTDs deployments in deep waters. A CTD is lowered on a winch all the way to the seabed and brought back up, so the deeper the water, the longer the ship has to sit still. +``` + This will create a folder/directory called `EXPEDITION_NAME` (or what you have changed this to) with a single file: `expedition.yaml`. This file contains details on the ship and instrument configurations, as well as the expedition schedule based on the sampling site coordinates (and, if specified, ports of departure/arrival) from your MFP export. The `--from-mfp` flag indicates that the exported route will be used, and `--start-date` is required alongside it. ## 5) Expedition scheduling & ship configuration @@ -149,7 +155,15 @@ Your Port of Departure and Port of Arrival (if set in MFP) appear as special way ``` ```{note} -It is important to ensure that the timings for each station are realistic. There must be enough time for the ship to travel to each site at the prescribed speed (10 knots). The expedition schedule will be automatically verified when you press _Save Changes_ in the planning tool. +It is important to ensure that the timings for each station are realistic. There must be enough time for the ship to travel to each site at the prescribed speed (10 knots), **and** enough time to actually carry out your deployments once you arrive. The expedition schedule will be automatically verified when you press _Save Changes_ in the planning tool. +``` + +```{important} +Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected. Time at a station is made up of slowing the ship down, carrying out the deployments, and getting back up to speed again. + +Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters, Argo floats and XBTs are simply released into the water and left behind, so they have no time cost apart from stopping the ship. A CTD is different, it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default + +If verification tells you that you would arrive somewhere too late, your waypoints with CTDs are the first place to look. See [here](../tutorials/working_with_expedition_yaml.md#how-long-the-ship-spends-at-each-waypoint) for exactly how this time is worked out. ``` ### Instrument selection diff --git a/docs/user-guide/quickstart.md b/docs/user-guide/quickstart.md index 257b04b4..c0f1a2c5 100644 --- a/docs/user-guide/quickstart.md +++ b/docs/user-guide/quickstart.md @@ -56,6 +56,12 @@ The `CruiseDataExport.xlsx` in the `virtualship init` command refers to the .xls The `--start-date` should be set to your intended departure date/time. MFP's estimated travel and station durations are then used to automatically fill in a time for each waypoint relative to this start date. ``` +```{important} +MFP usually allows a default of **45 minutes** at each station, and that default is what determines the waypoint times written into your `expedition.yaml`. That may well be enough for your expedition, but it may not be, so be prepared to adjust the times once you have chosen your instruments. + +Watch out in particular for **CTDs in deep water**. A CTD is winched all the way down and hauled back up, so the time it takes depends on the depth: a cast at a station in more than 2000 m of water takes over an hour on the winch alone, which does not fit inside a 45 minute allowance. VirtualShip will tell you if your schedule does not add up when you come to run it. See [here](./tutorials/working_with_expedition_yaml.md#how-long-the-ship-spends-at-each-waypoint) for how this time is calculated. +``` + This will create a folder/directory called `EXPEDITION_NAME` with a single file: `expedition.yaml` containing details on the ship and instrument configurations, as well as the expedition schedule based on the sampling site coordinates (and, if specified, ports of departure/arrival) from your MFP export. The `--from-mfp` flag indicates that the exported route will be used, and `--start-date` is required alongside it. ```{note} diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index f6ef699c..d7a492e7 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -40,6 +40,7 @@ schedule: # <-- 1. expedition schedule section location: latitude: 45.604174 longitude: -43.886739 + sea_depth_meter: -4400.0 time: 1998-03-08 03:37:00 # Waypoint 2 - instrument: @@ -49,6 +50,7 @@ schedule: # <-- 1. expedition schedule section location: latitude: 48.185988 longitude: -32.988302 + sea_depth_meter: -3200.0 time: 1998-03-10 03:05:00 # Port of Arrival - location: @@ -75,6 +77,8 @@ instruments_config: # <-- 2. instrument configuration section # ship_config: # <-- 3. ship configuration section ship_speed_knots: 10.0 + deceleration_time_minutes: 15.0 + acceleration_time_minutes: 15.0 ``` ```{note} @@ -113,9 +117,7 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -##### Comments - -You may notice comments such as `# Port of Departure`, `# Waypoint 1`, `# Waypoint 2`, etc. next to each entry in the `waypoints` list, as in the example above. These are added automatically whenever `virtualship init` or `virtualship plan` writes the file, purely to aid readability. They are not required when editing the file by hand and won't be automatically added/renumbered by doing so. +- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint. This value matters because it determines how a long a deployment takes if there is a depth dependence, e.g. for a CTD that has to be lowered and raised. If you leave it as `null`, VirtualShip doesn't know how deep the water is and assumes the CTD descends all the way to its configured `max_depth_meter`. That is the slowest possible cast, so your schedule is verified against a worst case rather than an optimistic one in the simulations. #### 2. `instruments_config` @@ -129,7 +131,7 @@ For most users, the most important instrument configuration setting to consider ctd_config: max_depth_meter: -2000.0 min_depth_meter: -11.0 - stationkeeping_time_minutes: 50.0 + winch_speed_meter_per_second: 1.0 sensors: - TEMPERATURE - SALINITY @@ -158,4 +160,18 @@ This section contains setting related to the ship itself, specifically: - **Ship speed (`ship_speed_knots`)**: The speed of the ship in knots (nautical miles per hour; where 1 knot = 1.852 km/h). Note in most cases this should be left as the default value unless there is a specific reason to change it. - +- **Deceleration and acceleration times (`deceleration_time_minutes`, `acceleration_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. + +### How long the ship spends at each waypoint + +When VirtualShip checks whether your schedule is achievable, it needs to know how long the ship is held stationary at each waypoint. That time is made up of three parts: + +```text +time at waypoint = deceleration + slowest deployment + acceleration +``` + +A few consequences are worth understanding, because they determine whether your schedule passes verification: + +- **Waypoints with no instruments cost nothing.** If `instrument` is `null` or an empty list, the ship does not slow down at all and simply sails past. + +- **Deployments happen side by side, so only the slowest one counts.** Different teams onboard work in parallel, so the total time is taken as whichever deployment takes longest. diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index b8cd907e..e55c43e3 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -62,6 +62,7 @@ schedule: location: latitude: 0.01 longitude: 0.01 + sea_depth_meter: null time: 1998-01-02 00:00:00 # Waypoint 2 - instrument: @@ -70,6 +71,7 @@ schedule: location: latitude: 0.02 longitude: 0.02 + sea_depth_meter: null time: 1998-01-03 01:00:00 # Waypoint 3 - instrument: @@ -77,6 +79,7 @@ schedule: location: latitude: 0.03 longitude: 0.03 + sea_depth_meter: null time: 1998-01-04 02:00:00 # Waypoint 4 - instrument: @@ -84,12 +87,14 @@ schedule: location: latitude: 0.04 longitude: 0.04 + sea_depth_meter: null time: 1998-01-05 03:00:00 # Waypoint 5 - instrument: [] location: latitude: 0.05 longitude: 0.05 + sea_depth_meter: null time: 1998-01-06 04:00:00 # Port of Arrival - location: From ef9d445a632776e7a47a66da843edf366f6d1ab6 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:58:32 +0200 Subject: [PATCH 14/39] use _field_validators helper --- src/virtualship/cli/_plan.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index 2122fdec..cbe7cf17 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -362,13 +362,7 @@ def compose(self) -> ComposeResult: yield Input( id=input_id, type=type_to_textual(get_field_type(ShipConfig, attr)), - validators=[ - Function( - validator, - f"INVALID: value must be {validator.__doc__.lower()}", - ) - for validator in group_validators(ShipConfig, attr) - ], + validators=_field_validators(ShipConfig, attr), classes="ship_speed_input", placeholder="minutes", value=str(current.total_seconds() / 60.0) From 6051b8b7af0746bb589ce0f6ea4d214dd8b17e27 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:53:53 +0200 Subject: [PATCH 15/39] update test for updated stationkeeping time calcs --- tests/cli/test_plan.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index cea9f4f1..820b73b0 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -93,7 +93,8 @@ async def _expand_waypoint(expedition_editor, pilot, index: int) -> WaypointWidg return widget -def _four_waypoint_schedule() -> list: +def _four_waypoint_schedule(hours_apart: int = 1) -> list: + """N.B. a full-depth CTD cast keeps the ship on station for ~1.5 h, so use hours_apart >= 2 for a schedule that passes verification (e.g. on save).""" return [ Port(location=None, time=None), Waypoint( @@ -103,12 +104,12 @@ def _four_waypoint_schedule() -> list: ), Waypoint( location=Location(0.01, 0.01), - time=datetime(2022, 1, 1, 1, 0, 0), + time=datetime(2022, 1, 1, hours_apart, 0, 0), instrument=["CTD"], ), Waypoint( location=Location(0.02, 0.02), - time=datetime(2022, 1, 1, 2, 0, 0), + time=datetime(2022, 1, 1, 2 * hours_apart, 0, 0), instrument=["CTD"], ), Port(location=None, time=None), @@ -645,7 +646,7 @@ async def test_waypoint_time_entry_and_adjust(tmp_path): @pytest.mark.asyncio async def test_save_shortcut_and_unsaved_changes_prompt(tmp_path): """ctrl+s saves, quitting with unsaved changes asks first, quitting after saving does not.""" - _make_expedition(tmp_path, _four_waypoint_schedule()) + _make_expedition(tmp_path, _four_waypoint_schedule(hours_apart=3)) app = PlanApp(path=tmp_path) async with app.run_test(size=(120, 100)) as pilot: From 295e26fd30acc20157d4612c7de84be1a6209728 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:47:42 +0200 Subject: [PATCH 16/39] enhance explanation about sea depth in config --- docs/user-guide/tutorials/working_with_expedition_yaml.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index a827c517..804c0236 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -117,10 +117,10 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint, as a negative number of metres (e.g. `-1500.0` for a site with a depth of 1500 m). +- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint, as a negative number of metres (e.g. `-1500.0` for a site with a depth of 1500 m). This is most relevant to CTDs where their deployment time has a depth dependence. It can be left as `null` if you don't know the depth, in which case e.g. CTD casts are assumed to reach the CTD's `max_depth_meter`, giving the slowest (worst-case) cast time. -```{tip} -Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. Note this is the opposite sign convention to the MFP export, which reports sea depth as a positive number. +```{note} +Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. This is the opposite sign convention to the MFP export, which reports sea depth as a positive number. ``` ##### Comments From df4a9c25f579809f25b1284a90f7614b25062784 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:15:25 +0200 Subject: [PATCH 17/39] remove unnecessary messaging and remove references to now deleted --- docs/user-guide/assignments/sail_the_ship.md | 2 +- docs/user-guide/quickstart.md | 2 +- .../tutorials/working_with_expedition_yaml.md | 14 -------------- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/docs/user-guide/assignments/sail_the_ship.md b/docs/user-guide/assignments/sail_the_ship.md index 367afce6..53141a07 100644 --- a/docs/user-guide/assignments/sail_the_ship.md +++ b/docs/user-guide/assignments/sail_the_ship.md @@ -163,7 +163,7 @@ Remember that MFP's default of 45 minutes per station may not be enough once you Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters, Argo floats and XBTs are simply released into the water and left behind, so they have no time cost apart from stopping the ship. A CTD is different, it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default -If verification tells you that you would arrive somewhere too late, your waypoints with CTDs are the first place to look. See [here](../tutorials/working_with_expedition_yaml.md#how-long-the-ship-spends-at-each-waypoint) for exactly how this time is worked out. +If verification tells you that you would arrive somewhere too late, your waypoints with CTDs are the first place to look. ``` ### Instrument selection diff --git a/docs/user-guide/quickstart.md b/docs/user-guide/quickstart.md index c0f1a2c5..111451f6 100644 --- a/docs/user-guide/quickstart.md +++ b/docs/user-guide/quickstart.md @@ -59,7 +59,7 @@ The `--start-date` should be set to your intended departure date/time. MFP's est ```{important} MFP usually allows a default of **45 minutes** at each station, and that default is what determines the waypoint times written into your `expedition.yaml`. That may well be enough for your expedition, but it may not be, so be prepared to adjust the times once you have chosen your instruments. -Watch out in particular for **CTDs in deep water**. A CTD is winched all the way down and hauled back up, so the time it takes depends on the depth: a cast at a station in more than 2000 m of water takes over an hour on the winch alone, which does not fit inside a 45 minute allowance. VirtualShip will tell you if your schedule does not add up when you come to run it. See [here](./tutorials/working_with_expedition_yaml.md#how-long-the-ship-spends-at-each-waypoint) for how this time is calculated. +Watch out in particular for **CTDs in deep water**. A CTD is winched all the way down and hauled back up, so the time it takes depends on the depth: a cast at a station in more than 2000 m of water takes over an hour on the winch alone, which does not fit inside a 45 minute allowance. VirtualShip will tell you if your schedule does not add up when you come to run it. ``` This will create a folder/directory called `EXPEDITION_NAME` with a single file: `expedition.yaml` containing details on the ship and instrument configurations, as well as the expedition schedule based on the sampling site coordinates (and, if specified, ports of departure/arrival) from your MFP export. The `--from-mfp` flag indicates that the exported route will be used, and `--start-date` is required alongside it. diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index d7a492e7..1c00eb56 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -161,17 +161,3 @@ This section contains setting related to the ship itself, specifically: - **Ship speed (`ship_speed_knots`)**: The speed of the ship in knots (nautical miles per hour; where 1 knot = 1.852 km/h). Note in most cases this should be left as the default value unless there is a specific reason to change it. - **Deceleration and acceleration times (`deceleration_time_minutes`, `acceleration_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. - -### How long the ship spends at each waypoint - -When VirtualShip checks whether your schedule is achievable, it needs to know how long the ship is held stationary at each waypoint. That time is made up of three parts: - -```text -time at waypoint = deceleration + slowest deployment + acceleration -``` - -A few consequences are worth understanding, because they determine whether your schedule passes verification: - -- **Waypoints with no instruments cost nothing.** If `instrument` is `null` or an empty list, the ship does not slow down at all and simply sails past. - -- **Deployments happen side by side, so only the slowest one counts.** Different teams onboard work in parallel, so the total time is taken as whichever deployment takes longest. From a2b708e1107834a59c4f7d79b046ae1eca6aa569 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:38:19 +0200 Subject: [PATCH 18/39] remove bathymetry vs planned depth warning (does not break simulations and is confusing to user) --- src/virtualship/models/expedition.py | 59 ++++------------------- tests/expedition/test_expedition.py | 72 ---------------------------- 2 files changed, 10 insertions(+), 121 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 6ed9a257..b1d28191 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -1,7 +1,6 @@ from __future__ import annotations import itertools -import warnings from collections.abc import Collection from datetime import datetime, timedelta from pathlib import Path @@ -103,9 +102,8 @@ def verify_schedule( self.schedule._check_structure(wps_in_use) - measured_depths = ( - {} if ignore_land_test else self.schedule._check_in_water(from_data) - ) + if not ignore_land_test: + self.schedule._check_in_water(from_data) # check that ship will arrive on time at each waypoint (in case no unexpected event happen) overrun = self._first_overrun(wps_in_use, ship_speed) @@ -122,29 +120,6 @@ def verify_schedule( "Have you ensured that your schedule includes sufficient time for taking measurements, e.g. CTD casts (in addition to the time it takes to sail between waypoints)?\n" ) - # the schedule works using the planned depths (e.g. from MFP), check it would also hold against the measured seabed (e.g. from bathy fieldset) - # warn when bringing in the measured sea depth would break the schedule - if measured_depths: - measured_overrun = self._first_overrun( - wps_in_use, ship_speed, measured_depths - ) - if measured_overrun is not None: - wp_i, wp_next, arrival_time = measured_overrun - # the deployment that overruns happens at the waypoint before the late arrival - wp = self.schedule.waypoints[wp_i] - wp_name = f"waypoint {_get_public_wp(wp_i, self.schedule.waypoints)}" - actual_depth = measured_depths[id(wp)] - warnings.warn( - f"The planned sea depth at {wp_name} (sea_depth_meter: {wp.sea_depth_meter}) " - f"does not match the actual sea depth at that location ({actual_depth:.1f}). " - f"The water there is deeper than planned, so deployments take longer and " - f"the ship would arrive too late at {self._describe_waypoint(wp_i, wp_next)} " - f"(projected {arrival_time.replace(microsecond=0)}, due {wp_next.time}).\n\n" - f"Please update 'sea_depth_meter' for {wp_name} to {actual_depth:.1f}, " - f"then adjust the schedule timings if needed.", - stacklevel=2, - ) - # mark this schedule as verified (so that subsequent stages of the workflow can proceed without re-verifying) self.schedule._verified = True @@ -154,7 +129,6 @@ def _first_overrun( self, wps_in_use: list[Port | Waypoint], ship_speed: float, - measured_depths: dict[int, float] | None = None, ) -> tuple[int, Port | Waypoint, datetime] | None: """Project the ship along the schedule and return the first waypoint it reaches late.""" time = wps_in_use[0].time @@ -165,8 +139,7 @@ def _first_overrun( for wp_i, (wp, wp_next) in enumerate( itertools.pairwise(wps_in_use), start=wps_in_use_offset ): - bathymetry = measured_depths.get(id(wp)) if measured_depths else None - stationkeeping_time = self.stationkeeping_time(wp, bathymetry) + stationkeeping_time = self.stationkeeping_time(wp) time_to_reach = _calc_sail_time( wp.location, @@ -193,26 +166,20 @@ def _describe_waypoint(self, wp_i: int, wp_next: Port | Waypoint) -> str: # +1 to get next return f"waypoint {_get_public_wp(wp_i + 1, self.schedule.waypoints)}" - def stationkeeping_time( - self, - waypoint: Port | Waypoint, - bathymetry: float | None = None, - ) -> timedelta: + def stationkeeping_time(self, waypoint: Port | Waypoint) -> timedelta: """ Calculate how long the ship is stationary at waypoint. Stationkeeping time = ship deceleration time + deployment time + ship acceleration time. Assumes insturments are deployed concurrently, so the longest deployment time is used. Empty waypoints (no instruments) cost nothing/ship sails through without slowing. - Optional `bathymetry` (measured seabed depth from simulation fieldset) overrides the waypoint's sea_depth_meter (estimated from MFP). """ instruments = self._wp_instruments(waypoint) if not instruments: return timedelta() deployment_time = max( - self._deployment_time(instrument, waypoint, bathymetry) - for instrument in instruments + self._deployment_time(instrument, waypoint) for instrument in instruments ) return ( @@ -225,16 +192,15 @@ def _deployment_time( self, instrument: InstrumentType, waypoint: Waypoint, - bathymetry: float | None = None, ) -> timedelta: """Time an instrument holds the ship on station.""" config = self.instruments_config.config_for(instrument) if config is None: return timedelta() - # a measured seabed depth (from simulation fieldset), when given, stands in for the waypoint's own estimate - seabed = bathymetry if bathymetry is not None else waypoint.sea_depth_meter - return get_instrument_class(instrument).deployment_time(config, seabed) + return get_instrument_class(instrument).deployment_time( + config, waypoint.sea_depth_meter + ) @staticmethod def _wp_instruments(waypoint: Port | Waypoint) -> list[InstrumentType]: @@ -386,8 +352,8 @@ def _check_structure(self, wps_in_use: list[Port | Waypoint]) -> None: f"Waypoint(s) {', '.join(invalid_labels)}: each waypoint should be timed after all previous waypoints", ) - def _check_in_water(self, from_data: Path | None = None) -> dict[int, float]: - """Check non-port waypoints sit in water rather than on land, return measured seabed depth at each (keyed by waypoint id).""" + def _check_in_water(self, from_data: Path | None = None) -> None: + """Check non-port waypoints sit in water rather than on land.""" try: bathymetry_field = _get_bathy_data(from_data=from_data).bathymetry except Exception as e: @@ -396,7 +362,6 @@ def _check_in_water(self, from_data: Path | None = None) -> dict[int, float]: ) from e land_waypoints = [] - measured_depths: dict[int, float] = {} for wp_i, wp in enumerate(self.waypoints): if isinstance(wp, Port): @@ -411,8 +376,6 @@ def _check_in_water(self, from_data: Path | None = None) -> dict[int, float]: ) if value == 0.0 or (isinstance(value, float) and np.isnan(value)): land_waypoints.append((public_wp, wp)) - else: - measured_depths[id(wp)] = float(np.ravel(value)[0]) except Exception as e: raise ScheduleError( f"Waypoint #{public_wp} at location {wp.location} could not be evaluated against bathymetry data. \n\n Original error: {e}" @@ -423,8 +386,6 @@ def _check_in_water(self, from_data: Path | None = None) -> dict[int, float]: f"The following waypoint(s) throw(s) error(s): {['#' + str(public_wp) + ' ' + str(wp) for (public_wp, wp) in land_waypoints]}\n\nINFO: They are likely on land (bathymetry data cannot be interpolated to their location(s)).\n" ) - return measured_depths - def _get_wps_in_use(self) -> list[Port | Waypoint]: """Return waypoints that are in use (i.e., have a specified time and location), i.e. excluding placeholder departure/arrival ports.""" start_slice = 0 if self.departure_port.is_in_use else 1 diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index 74817dcb..d44e8cd5 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -130,17 +130,6 @@ def test_stationkeeping_time_capped_by_sea_depth(expedition) -> None: ) -def test_stationkeeping_time_bathymetry_overrides_waypoint_estimate(expedition) -> None: - """A measured seabed depth (from simulation fieldset) is used in place of the waypoint's estimate from MFP.""" - waypoint = Waypoint( - location=Location(0, 0), instrument=InstrumentType.CTD, sea_depth_meter=-100.0 - ) - - assert expedition.stationkeeping_time( - waypoint, bathymetry=-800.0 - ) > expedition.stationkeeping_time(waypoint) - - def test_stationkeeping_time_single_instrument(expedition) -> None: """A bare (non-list) InstrumentType value is handled the same as a single element list.""" bare = Waypoint( @@ -214,67 +203,6 @@ def test_verify_on_land(base_expedition): ) -def _verify_against_deep_seabed(base_expedition, time_allowed_after_ctd): - """ - Verify a schedule whose CTD station is planned at 100 m but sits on a 3000 m seabed. - - At the planned depth (e.g. from MFP) the ship is held approx. 33 min; at the measured depth (e.g. from bathy fieldset; capped by the CTD's 2000 m max) approx.96 min. - The final leg to port takes approx. 36 min at 10 knots. - """ - lat = lon = np.array([0.0, 1.0, 2.0]) - ds_bathymetry = xr.Dataset( - {"deptho": (("lat", "lon"), np.full((3, 3), -3000.0))}, - coords={ - "lon": (("lon"), lon, {"units": "degrees_east"}), - "lat": (("lat"), lat, {"units": "degrees_north"}), - }, - ) - bathymetry_fieldset = parcels.FieldSet.from_sgrid_conventions( - parcels.convert.copernicusmarine_to_sgrid( - fields={"bathymetry": ds_bathymetry["deptho"]} - ) - ) - - ctd_time = datetime(2022, 1, 1, 8, 0, 0) - schedule = Schedule( - waypoints=[ - Port(location=Location(0.0, 0.0), time=datetime(2022, 1, 1, 0, 0, 0)), - Waypoint( - location=Location(0.0, 1.0), - time=ctd_time, - instrument=[InstrumentType.CTD], - sea_depth_meter=-100.0, - ), - Port(location=Location(0.0, 1.1), time=ctd_time + time_allowed_after_ctd), - ] - ) - - with patch( - "virtualship.models.expedition._get_bathy_data", - return_value=bathymetry_fieldset, - ): - _with_schedule(base_expedition, schedule).verify_schedule( - ignore_land_test=False, from_data=None - ) - - -def test_verify_warns_when_measured_bathymetry_breaks_schedule(base_expedition): - """A schedule that only fits at the planned depth warns that the real seabed breaks it.""" - with pytest.warns(UserWarning, match="actual sea depth") as record: - _verify_against_deep_seabed(base_expedition, timedelta(minutes=90)) - - # tells the user which value to put in the schedule - message = str(record[0].message) - assert "update 'sea_depth_meter' for waypoint 1 to -3000.0" in message - - -def test_verify_silent_when_measured_bathymetry_still_fits(base_expedition, recwarn): - """A deeper real seabed is not worth a warning if the schedule still has room for it.""" - _verify_against_deep_seabed(base_expedition, timedelta(hours=3)) - - assert not [w for w in recwarn if "actual sea depth" in str(w.message)] - - @pytest.mark.parametrize( "waypoints,error,match", [ From a197993ef7aaedbf245a2cc42939ea572d573a08 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:45:12 +0200 Subject: [PATCH 19/39] improve docs wording --- docs/user-guide/tutorials/working_with_expedition_yaml.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index 804c0236..3754093f 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -117,7 +117,7 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint, as a negative number of metres (e.g. `-1500.0` for a site with a depth of 1500 m). This is most relevant to CTDs where their deployment time has a depth dependence. It can be left as `null` if you don't know the depth, in which case e.g. CTD casts are assumed to reach the CTD's `max_depth_meter`, giving the slowest (worst-case) cast time. +- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint, as a negative number of metres (e.g. `-1500.0` for a site with a depth of 1500 m). This is most relevant to CTDs where their deployment time has a depth dependence. It can be left as `null` if you don't know the depth, in which case e.g. CTD casts are assumed to reach the CTD's `max_depth_meter`, giving the slowest (worst-case) cast time. It will be automatically filled in by `virtualship init` if you use the `--from-mfp` flag. ```{note} Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. This is the opposite sign convention to the MFP export, which reports sea depth as a positive number. @@ -125,7 +125,7 @@ Setting `sea_depth_meter` to a positive number will be rejected. Depths througho ##### Comments -You may notice comments such as `# Port of Departure`, `# Waypoint 1`, `# Waypoint 2`, etc. next to each entry in the `waypoints` list, as in the example above. These are added automatically whenever `virtualship init` or `virtualship plan` writes the file, purely to aid readability. They are not required when editing the file by hand and won't be automatically added/renumbered when doing so. +You may notice comments such as `# Port of Departure`, `# Waypoint 1`, `# Waypoint 2`, etc. next to each entry in the `waypoints` list, as in the example above. These are added automatically whenever `virtualship init` or `virtualship plan` writes the file, purely to improve readability. They are not required when editing the file by hand and won't be automatically added/renumbered when doing so. #### 2. `instruments_config` From 9b9d755365d5465531321138208f987a9a0acc3b Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:48:40 +0200 Subject: [PATCH 20/39] add ship deceleration and acceleration times to the plan tool --- src/virtualship/cli/_plan.py | 34 ++++++++++++++++++- tests/cli/test_plan.py | 64 +++++++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index c289e02a..cbe7cf17 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -352,6 +352,24 @@ def compose(self) -> ComposeResult: ) yield Label("", id="validation-failure-label-speed", classes="-hidden") + with Horizontal(classes="ship_speed"): + for attr, label, input_id in ( + ("deceleration_time", "Decelerate (mins):", "deceleration"), + ("acceleration_time", "Accelerate (mins):", "acceleration"), + ): + current = getattr(self.expedition.ship_config, attr, None) + yield Label(f"[b]{label}[/b]") + yield Input( + id=input_id, + type=type_to_textual(get_field_type(ShipConfig, attr)), + validators=_field_validators(ShipConfig, attr), + classes="ship_speed_input", + placeholder="minutes", + value=str(current.total_seconds() / 60.0) + if current + else "", + ) + with Horizontal(classes="ts-section"): yield Label("[b]Onboard Temperature/Salinity:[/b]") yield Switch( @@ -541,10 +559,24 @@ def _update_ship_speed(self): attr = "ship_speed_knots" field_type = get_field_type(type(self.expedition.ship_config), attr) value = field_type(self.query_one("#speed").value) + + # entered in minutes, stored as timedeltas + times = { + attr_name: datetime.timedelta( + minutes=float(self.query_one(f"#{input_id}").value) + ) + for attr_name, input_id in ( + ("deceleration_time", "deceleration"), + ("acceleration_time", "acceleration"), + ) + } + ShipConfig.model_validate( - {**self.expedition.ship_config.model_dump(), attr: value} + {**self.expedition.ship_config.model_dump(), attr: value, **times} ) self.expedition.ship_config.ship_speed_knots = value + for attr_name, time_value in times.items(): + setattr(self.expedition.ship_config, attr_name, time_value) def _update_instrument_configs(self): instruments_config = self.expedition.instruments_config diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 820b73b0..64fca32a 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -215,7 +215,11 @@ async def test_UI_changes(tmp_path): assert saved_wps[2].location.lon == float(NEW_LON) assert saved_wps[1].instrument == [InstrumentType.XBT] - # check schedule.verify() methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) + # the ship's slow-down / speed-up times survive a round trip through the UI + assert saved_expedition["ship_config"]["deceleration_time_minutes"] == 15.0 + assert saved_expedition["ship_config"]["acceleration_time_minutes"] == 15.0 + + # check expedition.verify_schedule() methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) invalid_speed = "0.0001" await simulate_input(pilot, ship_speed_input, invalid_speed) await pilot.click(save_button) @@ -225,6 +229,64 @@ async def test_UI_changes(tmp_path): assert "*** Error saving changes ***" in args[0] +@pytest.mark.asyncio +async def test_ship_decel_accel_editable_and_saved(tmp_path): + """The ship's deceleration and acceleration times can be edited in the UI and are written back.""" + _make_expedition( + tmp_path, + [ + Port(location=None, time=None), + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 0, 0, 0), + instrument=["CTD"], + ), + Waypoint( + location=Location(0.01, 0.01), + time=datetime(2022, 1, 1, 6, 0, 0), + instrument=None, + ), + Port(location=None, time=None), + ], + ) + + app = PlanApp(path=tmp_path) + async with app.run_test(size=(160, 140)) as pilot: + await pilot.pause(0.5) + plan_screen = pilot.app.screen + plan_screen.notify = MagicMock() + expedition_editor = plan_screen.query_one(ExpeditionEditor) + + speed_collapsible = expedition_editor.query_one( + "#speed_collapsible", Collapsible + ) + speed_collapsible.collapsed = False + await pilot.pause() + + await simulate_input( + pilot, expedition_editor.query_one("#deceleration", Input), "20" + ) + await simulate_input( + pilot, expedition_editor.query_one("#acceleration", Input), "25" + ) + + await pilot.click(plan_screen.query_one("#save_button", Button)) + await pilot.pause(0.5) + + calls = plan_screen.notify.call_args_list + assert any( + call[0][0] == "Changes saved successfully" + and call[1].get("severity") == "information" + for call in calls + ) + + with open(tmp_path / EXPEDITION) as f: + saved = yaml.safe_load(f) + + assert saved["ship_config"]["deceleration_time_minutes"] == 20.0 + assert saved["ship_config"]["acceleration_time_minutes"] == 25.0 + + @pytest.mark.asyncio async def test_UI_opens_with_null_time_and_instrument(tmp_path): """Test that the UI opens correctly when waypoints have time: null and instrument: null.""" From dc3a5764029efe23829abcfe027fbc6393d7a5e3 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:29:31 +0200 Subject: [PATCH 21/39] handle MFP export format, e.g. ~ symbols --- src/virtualship/cli/_initialise.py | 39 +++++++++++++++++++++-- tests/cli/test_initialise.py | 51 +++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/src/virtualship/cli/_initialise.py b/src/virtualship/cli/_initialise.py index 44272dbc..8a58c618 100644 --- a/src/virtualship/cli/_initialise.py +++ b/src/virtualship/cli/_initialise.py @@ -67,6 +67,37 @@ def _initialise( click.echo(f"Created '{expedition.name}' at {path}.") +def _parse_mfp_sea_depth(value, waypoint_name: str) -> float | None: + """ + Sea depth from an MFP export, as a negative elevation in metres. + + MFP reports depth as a positive number of metres (usually with an "~"). + Returns None (i.e. unknown depth) for blank, unreadable, or non-positive values. + """ + if pd.isna(value): + return None + + match = re.fullmatch(r"~?\s*(-?\d+(?:\.\d+)?)\s*m?", str(value).strip()) + if match is None: + warnings.warn( + f"Could not read the sea depth '{value}' for {waypoint_name}, so it has been left unset. {ERR_SUPPLEMENT}", + stacklevel=2, + ) + return None + + depth = float(match.group(1)) + if depth <= 0: + warnings.warn( + f"{waypoint_name} has a sea depth of '{value}' (at or above sea level, so possibly on land), so it has been left unset.", + stacklevel=2, + ) + return None + + return ( + -depth + ) # MFP reports sea depth as positive, but here we use negative elevations + + def _mfp_to_yaml(file_path: Path, start_date: str, output_path: Path): """Generates an expedition.yaml file from MFP Excel export.""" mfp_data = _validate_mfp_data(file_path) @@ -102,9 +133,11 @@ def _mfp_to_yaml(file_path: Path, start_date: str, output_path: Path): has_latlon = lat is not None and lon is not None waypoints.append(Port(location=loc, time=time_val if has_latlon else None)) else: - sea_depth = ( - None if pd.isna(row["Sea Depth"]) else -abs(float(row["Sea Depth"])) - ) # MFP reports sea depth as positive, but here we use negative elevations + # numbered as in the generated expedition.yaml's '# Waypoint N' comments + waypoint_number = sum(isinstance(wp, Waypoint) for wp in waypoints) + 1 + sea_depth = _parse_mfp_sea_depth( + row["Sea Depth"], f"Waypoint {waypoint_number}" + ) waypoints.append( Waypoint( instrument=None, diff --git a/tests/cli/test_initialise.py b/tests/cli/test_initialise.py index 50273c33..566700e8 100644 --- a/tests/cli/test_initialise.py +++ b/tests/cli/test_initialise.py @@ -1,7 +1,11 @@ import pandas as pd import pytest -from virtualship.cli._initialise import _load_mfp_export, _mfp_to_yaml +from virtualship.cli._initialise import ( + _load_mfp_export, + _mfp_to_yaml, + _parse_mfp_sea_depth, +) from virtualship.models import Expedition, Port, Waypoint from virtualship.utils import _get_example_expedition @@ -202,6 +206,51 @@ def test_mfp_sea_depth_absent_leaves_waypoint_depth_unset(tmp_path): assert all(wp.sea_depth_meter is None for wp in waypoints) +def test_mfp_sea_depth_read_from_export_text_format(tmp_path): + """MFP exports write sea depth as text such as '~2865m', which is read as a negative depth.""" + df = valid_mfp_data() + df["Sea Depth"] = [f"~{d}m" for d in df["Sea Depth"]] + mfp_file = tmp_path / "text_depths.xlsx" + df.to_excel(mfp_file, index=False) + + yaml_output_path = tmp_path / "expedition.yaml" + _mfp_to_yaml(mfp_file, "2023-10-20 01:00:00", yaml_output_path) + + expedition = Expedition.from_yaml(yaml_output_path) + waypoints = [wp for wp in expedition.schedule.waypoints if isinstance(wp, Waypoint)] + assert [wp.sea_depth_meter for wp in waypoints] == [-200.0, -300.0, -400.0] + + +@pytest.mark.parametrize( + "value,expected", + [ + ("~2865m", -2865.0), + ("~ 73 m", -73.0), + ("2865", -2865.0), + (2865, -2865.0), + (152.5, -152.5), + (None, None), + ], +) +def test_parse_mfp_sea_depth(value, expected): + assert _parse_mfp_sea_depth(value, "Waypoint 1") == expected + + +@pytest.mark.parametrize( + "value,match", + [ + ("~-9m", "at or above sea level"), + ("~0m", "at or above sea level"), + ("deep", "Could not read the sea depth"), + ], +) +def test_parse_mfp_sea_depth_unusable_left_unset_with_warning(value, match): + """Unreadable or non-positive (on/above sea level) depths are left unset, with a warning naming the waypoint.""" + with pytest.warns(UserWarning, match=match) as record: + assert _parse_mfp_sea_depth(value, "Waypoint 1") is None + assert "Waypoint 1" in str(record[0].message) + + @pytest.mark.parametrize( "fixture_name,error,match", [ From 49b301f083d016164dc87da01b24b1610568c43f Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:32:56 +0200 Subject: [PATCH 22/39] revert plan tool changes, need to consider how want to present this to user --- src/virtualship/cli/_plan.py | 34 +------------------ tests/cli/test_plan.py | 64 +----------------------------------- 2 files changed, 2 insertions(+), 96 deletions(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index cbe7cf17..c289e02a 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -352,24 +352,6 @@ def compose(self) -> ComposeResult: ) yield Label("", id="validation-failure-label-speed", classes="-hidden") - with Horizontal(classes="ship_speed"): - for attr, label, input_id in ( - ("deceleration_time", "Decelerate (mins):", "deceleration"), - ("acceleration_time", "Accelerate (mins):", "acceleration"), - ): - current = getattr(self.expedition.ship_config, attr, None) - yield Label(f"[b]{label}[/b]") - yield Input( - id=input_id, - type=type_to_textual(get_field_type(ShipConfig, attr)), - validators=_field_validators(ShipConfig, attr), - classes="ship_speed_input", - placeholder="minutes", - value=str(current.total_seconds() / 60.0) - if current - else "", - ) - with Horizontal(classes="ts-section"): yield Label("[b]Onboard Temperature/Salinity:[/b]") yield Switch( @@ -559,24 +541,10 @@ def _update_ship_speed(self): attr = "ship_speed_knots" field_type = get_field_type(type(self.expedition.ship_config), attr) value = field_type(self.query_one("#speed").value) - - # entered in minutes, stored as timedeltas - times = { - attr_name: datetime.timedelta( - minutes=float(self.query_one(f"#{input_id}").value) - ) - for attr_name, input_id in ( - ("deceleration_time", "deceleration"), - ("acceleration_time", "acceleration"), - ) - } - ShipConfig.model_validate( - {**self.expedition.ship_config.model_dump(), attr: value, **times} + {**self.expedition.ship_config.model_dump(), attr: value} ) self.expedition.ship_config.ship_speed_knots = value - for attr_name, time_value in times.items(): - setattr(self.expedition.ship_config, attr_name, time_value) def _update_instrument_configs(self): instruments_config = self.expedition.instruments_config diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 64fca32a..168cabea 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -215,11 +215,7 @@ async def test_UI_changes(tmp_path): assert saved_wps[2].location.lon == float(NEW_LON) assert saved_wps[1].instrument == [InstrumentType.XBT] - # the ship's slow-down / speed-up times survive a round trip through the UI - assert saved_expedition["ship_config"]["deceleration_time_minutes"] == 15.0 - assert saved_expedition["ship_config"]["acceleration_time_minutes"] == 15.0 - - # check expedition.verify_schedule() methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) + # check verifiction methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) invalid_speed = "0.0001" await simulate_input(pilot, ship_speed_input, invalid_speed) await pilot.click(save_button) @@ -229,64 +225,6 @@ async def test_UI_changes(tmp_path): assert "*** Error saving changes ***" in args[0] -@pytest.mark.asyncio -async def test_ship_decel_accel_editable_and_saved(tmp_path): - """The ship's deceleration and acceleration times can be edited in the UI and are written back.""" - _make_expedition( - tmp_path, - [ - Port(location=None, time=None), - Waypoint( - location=Location(0, 0), - time=datetime(2022, 1, 1, 0, 0, 0), - instrument=["CTD"], - ), - Waypoint( - location=Location(0.01, 0.01), - time=datetime(2022, 1, 1, 6, 0, 0), - instrument=None, - ), - Port(location=None, time=None), - ], - ) - - app = PlanApp(path=tmp_path) - async with app.run_test(size=(160, 140)) as pilot: - await pilot.pause(0.5) - plan_screen = pilot.app.screen - plan_screen.notify = MagicMock() - expedition_editor = plan_screen.query_one(ExpeditionEditor) - - speed_collapsible = expedition_editor.query_one( - "#speed_collapsible", Collapsible - ) - speed_collapsible.collapsed = False - await pilot.pause() - - await simulate_input( - pilot, expedition_editor.query_one("#deceleration", Input), "20" - ) - await simulate_input( - pilot, expedition_editor.query_one("#acceleration", Input), "25" - ) - - await pilot.click(plan_screen.query_one("#save_button", Button)) - await pilot.pause(0.5) - - calls = plan_screen.notify.call_args_list - assert any( - call[0][0] == "Changes saved successfully" - and call[1].get("severity") == "information" - for call in calls - ) - - with open(tmp_path / EXPEDITION) as f: - saved = yaml.safe_load(f) - - assert saved["ship_config"]["deceleration_time_minutes"] == 20.0 - assert saved["ship_config"]["acceleration_time_minutes"] == 25.0 - - @pytest.mark.asyncio async def test_UI_opens_with_null_time_and_instrument(tmp_path): """Test that the UI opens correctly when waypoints have time: null and instrument: null.""" From 8cdcb14f82a25d6c411d624d07895a06e2be9f71 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:37:34 +0200 Subject: [PATCH 23/39] add deceleration and acceleration fields to plan TUI --- src/virtualship/cli/_plan.py | 57 +++++++++++++++++++++++++++++- tests/cli/test_plan.py | 67 ++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index c289e02a..a66d4c77 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -195,6 +195,9 @@ def _raise_logged_unexpected(e: Exception, path, context_message: str): DEFAULT_TS_CONFIG = {"period_minutes": 5.0} +# ship config timings, edited in minutes in their own (collapsed by default) section +SHIP_TIME_ATTRS = ("deceleration_time", "acceleration_time") + DEFAULT_ADCP_CONFIG = { "num_bins": 40, "period_minutes": 5.0, @@ -352,6 +355,27 @@ def compose(self) -> ComposeResult: ) yield Label("", id="validation-failure-label-speed", classes="-hidden") + # advanced settings, which most users can leave as the defaults + with Collapsible( + title="Ship deceleration & acceleration times", + id="ship_advanced_collapsible", + collapsed=True, + ): + with Container(classes="instrument-config"): + for attr in SHIP_TIME_ATTRS: + yield Label( + f"{attr.replace('_', ' ').capitalize()} (minutes):" + ) + yield Input( + id=attr, + type=type_to_textual(get_field_type(ShipConfig, attr)), + validators=_field_validators(ShipConfig, attr), + value=_config_display_value( + ship_config, {"name": attr, "minutes": True} + ), + ) + yield _failure_label(attr) + with Horizontal(classes="ts-section"): yield Label("[b]Onboard Temperature/Salinity:[/b]") yield Switch( @@ -541,10 +565,21 @@ def _update_ship_speed(self): attr = "ship_speed_knots" field_type = get_field_type(type(self.expedition.ship_config), attr) value = field_type(self.query_one("#speed").value) + + # entered in minutes, stored as timedeltas + times = { + time_attr: datetime.timedelta( + minutes=float(self.query_one(f"#{time_attr}").value) + ) + for time_attr in SHIP_TIME_ATTRS + } + ShipConfig.model_validate( - {**self.expedition.ship_config.model_dump(), attr: value} + {**self.expedition.ship_config.model_dump(), attr: value, **times} ) self.expedition.ship_config.ship_speed_knots = value + for time_attr, time_value in times.items(): + setattr(self.expedition.ship_config, time_attr, time_value) def _update_instrument_configs(self): instruments_config = self.expedition.instruments_config @@ -1308,6 +1343,26 @@ class PlanApp(App): margin: 0 4; } + #ship_advanced_collapsible { + margin: 2 0; + padding-left: 0; + padding-bottom: 0; + border-top: none; + background: transparent; + } + + #ship_advanced_collapsible > CollapsibleTitle { + padding: 0; + } + + #ship_advanced_collapsible > Contents { + padding-top: 0; + } + + #ship_advanced_collapsible .instrument-config { + margin: 0 1; + } + .instrument-config { margin: 1; padding: 0 2; diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 168cabea..5d360702 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -215,6 +215,10 @@ async def test_UI_changes(tmp_path): assert saved_wps[2].location.lon == float(NEW_LON) assert saved_wps[1].instrument == [InstrumentType.XBT] + # the ship's slow-down / speed-up times survive a round trip through the UI, untouched + assert saved_expedition["ship_config"]["deceleration_time_minutes"] == 15.0 + assert saved_expedition["ship_config"]["acceleration_time_minutes"] == 15.0 + # check verifiction methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) invalid_speed = "0.0001" await simulate_input(pilot, ship_speed_input, invalid_speed) @@ -225,6 +229,69 @@ async def test_UI_changes(tmp_path): assert "*** Error saving changes ***" in args[0] +@pytest.mark.asyncio +async def test_ship_decel_accel_editable_and_saved(tmp_path): + """The ship's deceleration and acceleration times sit in their own collapsed section, and can be edited there and saved.""" + _make_expedition( + tmp_path, + [ + Port(location=None, time=None), + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 0, 0, 0), + instrument=["CTD"], + ), + Waypoint( + location=Location(0.01, 0.01), + time=datetime(2022, 1, 1, 6, 0, 0), + instrument=None, + ), + Port(location=None, time=None), + ], + ) + + app = PlanApp(path=tmp_path) + async with app.run_test(size=(160, 140)) as pilot: + await pilot.pause(0.5) + plan_screen = pilot.app.screen + plan_screen.notify = MagicMock() + expedition_editor = plan_screen.query_one(ExpeditionEditor) + + # collapsed by default + advanced = expedition_editor.query_one( + "#ship_advanced_collapsible", Collapsible + ) + assert advanced.collapsed + assert expedition_editor.query_one("#deceleration_time", Input).value == "15.0" + + advanced.collapsed = False + await pilot.pause() + + await simulate_input( + pilot, expedition_editor.query_one("#deceleration_time", Input), "20" + ) + await simulate_input( + pilot, expedition_editor.query_one("#acceleration_time", Input), "25" + ) + assert expedition_editor.has_unsaved_changes() + + await pilot.click(plan_screen.query_one("#save_button", Button)) + await pilot.pause(0.5) + + calls = plan_screen.notify.call_args_list + assert any( + call[0][0] == "Changes saved successfully" + and call[1].get("severity") == "information" + for call in calls + ) + + with open(tmp_path / EXPEDITION) as f: + saved = yaml.safe_load(f) + + assert saved["ship_config"]["deceleration_time_minutes"] == 20.0 + assert saved["ship_config"]["acceleration_time_minutes"] == 25.0 + + @pytest.mark.asyncio async def test_UI_opens_with_null_time_and_instrument(tmp_path): """Test that the UI opens correctly when waypoints have time: null and instrument: null.""" From 28e8a21b57dddcb6cec31ddb76c51ea0cf52b429 Mon Sep 17 00:00:00 2001 From: Jamie Atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:02:37 +0200 Subject: [PATCH 24/39] Apply suggestion from @erikvansebille Co-authored-by: Erik van Sebille --- docs/user-guide/assignments/sail_the_ship.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/assignments/sail_the_ship.md b/docs/user-guide/assignments/sail_the_ship.md index 53141a07..5d4b8a33 100644 --- a/docs/user-guide/assignments/sail_the_ship.md +++ b/docs/user-guide/assignments/sail_the_ship.md @@ -161,7 +161,7 @@ It is important to ensure that the timings for each station are realistic. There ```{important} Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected. Time at a station is made up of slowing the ship down, carrying out the deployments, and getting back up to speed again. -Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters, Argo floats and XBTs are simply released into the water and left behind, so they have no time cost apart from stopping the ship. A CTD is different, it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default +Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters, Argo floats and XBTs are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. A CTD is different, it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default If verification tells you that you would arrive somewhere too late, your waypoints with CTDs are the first place to look. ``` From 171de72bcf617755429ea6fce6be51da7634cce7 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:05:17 +0200 Subject: [PATCH 25/39] explicit about default times for slow down and speed up --- docs/user-guide/assignments/sail_the_ship.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/assignments/sail_the_ship.md b/docs/user-guide/assignments/sail_the_ship.md index 53141a07..04d348a7 100644 --- a/docs/user-guide/assignments/sail_the_ship.md +++ b/docs/user-guide/assignments/sail_the_ship.md @@ -159,7 +159,7 @@ It is important to ensure that the timings for each station are realistic. There ``` ```{important} -Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected. Time at a station is made up of slowing the ship down, carrying out the deployments, and getting back up to speed again. +Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected. Time at a station is made up of slowing the ship down (15 minutes by default), carrying out the deployments (which depends on which instruments you deploy), and getting back up to speed again (15 minutes by default). Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters, Argo floats and XBTs are simply released into the water and left behind, so they have no time cost apart from stopping the ship. A CTD is different, it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default From e7174b357bbc640aab8a1ef33a784cac89b5ca5f Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:18:43 +0200 Subject: [PATCH 26/39] "deceleration"/"acceleration" -> "slow-down"/"speed-up" times --- .../tutorials/working_with_expedition_yaml.md | 6 ++--- src/virtualship/cli/_plan.py | 4 ++-- src/virtualship/models/expedition.py | 24 ++++++++++--------- src/virtualship/static/expedition.yaml | 4 ++-- tests/cli/test_plan.py | 18 +++++++------- .../expedition/expedition_dir/expedition.yaml | 4 ++-- tests/expedition/test_expedition.py | 4 +++- tests/expedition/test_simulate_schedule.py | 4 ++-- .../make_realistic/problems/test_simulator.py | 4 ++-- 9 files changed, 39 insertions(+), 33 deletions(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index 1c00eb56..b8338126 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -77,8 +77,8 @@ instruments_config: # <-- 2. instrument configuration section # ship_config: # <-- 3. ship configuration section ship_speed_knots: 10.0 - deceleration_time_minutes: 15.0 - acceleration_time_minutes: 15.0 + station_slowdown_time_minutes: 15.0 + station_speedup_time_minutes: 15.0 ``` ```{note} @@ -160,4 +160,4 @@ This section contains setting related to the ship itself, specifically: - **Ship speed (`ship_speed_knots`)**: The speed of the ship in knots (nautical miles per hour; where 1 knot = 1.852 km/h). Note in most cases this should be left as the default value unless there is a specific reason to change it. -- **Deceleration and acceleration times (`deceleration_time_minutes`, `acceleration_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. +- **Station slow-down and speed-up times (`station_slowdown_time_minutes`, `station_speedup_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index a66d4c77..da4b91d9 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -196,7 +196,7 @@ def _raise_logged_unexpected(e: Exception, path, context_message: str): DEFAULT_TS_CONFIG = {"period_minutes": 5.0} # ship config timings, edited in minutes in their own (collapsed by default) section -SHIP_TIME_ATTRS = ("deceleration_time", "acceleration_time") +SHIP_TIME_ATTRS = ("station_slowdown_time", "station_speedup_time") DEFAULT_ADCP_CONFIG = { "num_bins": 40, @@ -357,7 +357,7 @@ def compose(self) -> ComposeResult: # advanced settings, which most users can leave as the defaults with Collapsible( - title="Ship deceleration & acceleration times", + title="Station slow-down & speed-up times", id="ship_advanced_collapsible", collapsed=True, ): diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 1bd6b21d..bdbc74e5 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -170,7 +170,7 @@ def stationkeeping_time(self, waypoint: Port | Waypoint) -> timedelta: """ Calculate how long the ship is stationary at waypoint. - Stationkeeping time = ship deceleration time + deployment time + ship acceleration time. + Stationkeeping time = slow-down time + deployment time + speed-up time. Assumes insturments are deployed concurrently, so the longest deployment time is used. Empty waypoints (no instruments) cost nothing/ship sails through without slowing. """ @@ -183,9 +183,9 @@ def stationkeeping_time(self, waypoint: Port | Waypoint) -> timedelta: ) return ( - self.ship_config.deceleration_time + self.ship_config.station_slowdown_time + deployment_time - + self.ship_config.acceleration_time + + self.ship_config.station_speedup_time ) def _deployment_time( @@ -262,14 +262,14 @@ class ShipConfig(pydantic.BaseModel): ship_speed_knots: float = pydantic.Field(gt=0.0) - deceleration_time: timedelta = pydantic.Field( - serialization_alias="deceleration_time_minutes", - validation_alias="deceleration_time_minutes", + station_slowdown_time: timedelta = pydantic.Field( + serialization_alias="station_slowdown_time_minutes", + validation_alias="station_slowdown_time_minutes", ge=timedelta(), ) - acceleration_time: timedelta = pydantic.Field( - serialization_alias="acceleration_time_minutes", - validation_alias="acceleration_time_minutes", + station_speedup_time: timedelta = pydantic.Field( + serialization_alias="station_speedup_time_minutes", + validation_alias="station_speedup_time_minutes", ge=timedelta(), ) @@ -277,11 +277,13 @@ class ShipConfig(pydantic.BaseModel): model_config = pydantic.ConfigDict(extra="forbid", populate_by_name=True) - @pydantic.field_serializer("deceleration_time", "acceleration_time") + @pydantic.field_serializer("station_slowdown_time", "station_speedup_time") def _serialize_minutes(self, value: timedelta, _info) -> float: return value.total_seconds() / 60.0 - @pydantic.field_validator("deceleration_time", "acceleration_time", mode="before") + @pydantic.field_validator( + "station_slowdown_time", "station_speedup_time", mode="before" + ) @classmethod def _validate_minutes(cls, value: int | float | timedelta) -> timedelta: return _validate_numeric_to_timedelta(value, "minutes") diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index e55c43e3..46835505 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -103,5 +103,5 @@ schedule: time: 1998-01-07 05:00:00 ship_config: ship_speed_knots: 10.0 - deceleration_time_minutes: 15.0 - acceleration_time_minutes: 15.0 + station_slowdown_time_minutes: 15.0 + station_speedup_time_minutes: 15.0 diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 5d360702..985c3bc6 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -216,8 +216,8 @@ async def test_UI_changes(tmp_path): assert saved_wps[1].instrument == [InstrumentType.XBT] # the ship's slow-down / speed-up times survive a round trip through the UI, untouched - assert saved_expedition["ship_config"]["deceleration_time_minutes"] == 15.0 - assert saved_expedition["ship_config"]["acceleration_time_minutes"] == 15.0 + assert saved_expedition["ship_config"]["station_slowdown_time_minutes"] == 15.0 + assert saved_expedition["ship_config"]["station_speedup_time_minutes"] == 15.0 # check verifiction methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) invalid_speed = "0.0001" @@ -231,7 +231,7 @@ async def test_UI_changes(tmp_path): @pytest.mark.asyncio async def test_ship_decel_accel_editable_and_saved(tmp_path): - """The ship's deceleration and acceleration times sit in their own collapsed section, and can be edited there and saved.""" + """The ship's station slow-down and speed-up times sit in their own collapsed section, and can be edited there and saved.""" _make_expedition( tmp_path, [ @@ -262,16 +262,18 @@ async def test_ship_decel_accel_editable_and_saved(tmp_path): "#ship_advanced_collapsible", Collapsible ) assert advanced.collapsed - assert expedition_editor.query_one("#deceleration_time", Input).value == "15.0" + assert ( + expedition_editor.query_one("#station_slowdown_time", Input).value == "15.0" + ) advanced.collapsed = False await pilot.pause() await simulate_input( - pilot, expedition_editor.query_one("#deceleration_time", Input), "20" + pilot, expedition_editor.query_one("#station_slowdown_time", Input), "20" ) await simulate_input( - pilot, expedition_editor.query_one("#acceleration_time", Input), "25" + pilot, expedition_editor.query_one("#station_speedup_time", Input), "25" ) assert expedition_editor.has_unsaved_changes() @@ -288,8 +290,8 @@ async def test_ship_decel_accel_editable_and_saved(tmp_path): with open(tmp_path / EXPEDITION) as f: saved = yaml.safe_load(f) - assert saved["ship_config"]["deceleration_time_minutes"] == 20.0 - assert saved["ship_config"]["acceleration_time_minutes"] == 25.0 + assert saved["ship_config"]["station_slowdown_time_minutes"] == 20.0 + assert saved["ship_config"]["station_speedup_time_minutes"] == 25.0 @pytest.mark.asyncio diff --git a/tests/expedition/expedition_dir/expedition.yaml b/tests/expedition/expedition_dir/expedition.yaml index 4141e899..c524a5bf 100644 --- a/tests/expedition/expedition_dir/expedition.yaml +++ b/tests/expedition/expedition_dir/expedition.yaml @@ -51,5 +51,5 @@ instruments_config: period_minutes: 5.0 ship_config: ship_speed_knots: 10.0 - deceleration_time_minutes: 15.0 - acceleration_time_minutes: 15.0 + station_slowdown_time_minutes: 15.0 + station_speedup_time_minutes: 15.0 diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index d44e8cd5..d7af2096 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -105,7 +105,9 @@ def test_stationkeeping_time_uses_slowest_deployment(expedition) -> None: * abs(-500.0 - ctd_config.min_depth_meter) / ctd_config.winch_speed_meter_per_second ) - expected = ship_config.deceleration_time + cast + ship_config.acceleration_time + expected = ( + ship_config.station_slowdown_time + cast + ship_config.station_speedup_time + ) assert expedition.stationkeeping_time(waypoint) == expected diff --git a/tests/expedition/test_simulate_schedule.py b/tests/expedition/test_simulate_schedule.py index 19d50fec..d8505a93 100644 --- a/tests/expedition/test_simulate_schedule.py +++ b/tests/expedition/test_simulate_schedule.py @@ -44,8 +44,8 @@ def test_time_in_minutes_in_ship_schedule() -> None: assert instruments_config.ship_underwater_st_config.period == timedelta(minutes=5) ship_config = Expedition.from_yaml(TEST_EXPEDITION_YAML).ship_config - assert ship_config.deceleration_time == timedelta(minutes=15) - assert ship_config.acceleration_time == timedelta(minutes=15) + assert ship_config.station_slowdown_time == timedelta(minutes=15) + assert ship_config.station_speedup_time == timedelta(minutes=15) def test_ship_path_inside_domain() -> None: diff --git a/tests/make_realistic/problems/test_simulator.py b/tests/make_realistic/problems/test_simulator.py index 5be80dd6..802785c2 100644 --- a/tests/make_realistic/problems/test_simulator.py +++ b/tests/make_realistic/problems/test_simulator.py @@ -59,8 +59,8 @@ def _make_simple_expedition( instruments = InstrumentsConfig() ship = ShipConfig( ship_speed_knots=10.0, - deceleration_time_minutes=15.0, - acceleration_time_minutes=15.0, + station_slowdown_time_minutes=15.0, + station_speedup_time_minutes=15.0, ) return Expedition( schedule=schedule, instruments_config=instruments, ship_config=ship From 677f905f48a6fc9aabb7af8eb35b0e18a9651297 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:22:31 +0200 Subject: [PATCH 27/39] "station" -> "waypoint" --- .../tutorials/working_with_expedition_yaml.md | 6 +++--- src/virtualship/cli/_plan.py | 4 ++-- src/virtualship/models/expedition.py | 20 +++++++++---------- src/virtualship/static/expedition.yaml | 4 ++-- tests/cli/test_plan.py | 17 ++++++++-------- .../expedition/expedition_dir/expedition.yaml | 4 ++-- tests/expedition/test_expedition.py | 2 +- tests/expedition/test_simulate_schedule.py | 4 ++-- .../make_realistic/problems/test_simulator.py | 4 ++-- 9 files changed, 33 insertions(+), 32 deletions(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index b8338126..689ce356 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -77,8 +77,8 @@ instruments_config: # <-- 2. instrument configuration section # ship_config: # <-- 3. ship configuration section ship_speed_knots: 10.0 - station_slowdown_time_minutes: 15.0 - station_speedup_time_minutes: 15.0 + waypoint_slowdown_time_minutes: 15.0 + waypoint_speedup_time_minutes: 15.0 ``` ```{note} @@ -160,4 +160,4 @@ This section contains setting related to the ship itself, specifically: - **Ship speed (`ship_speed_knots`)**: The speed of the ship in knots (nautical miles per hour; where 1 knot = 1.852 km/h). Note in most cases this should be left as the default value unless there is a specific reason to change it. -- **Station slow-down and speed-up times (`station_slowdown_time_minutes`, `station_speedup_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. +- **Waypoint slow-down and speed-up times (`waypoint_slowdown_time_minutes`, `waypoint_speedup_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index da4b91d9..7cc8fa48 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -196,7 +196,7 @@ def _raise_logged_unexpected(e: Exception, path, context_message: str): DEFAULT_TS_CONFIG = {"period_minutes": 5.0} # ship config timings, edited in minutes in their own (collapsed by default) section -SHIP_TIME_ATTRS = ("station_slowdown_time", "station_speedup_time") +SHIP_TIME_ATTRS = ("waypoint_slowdown_time", "waypoint_speedup_time") DEFAULT_ADCP_CONFIG = { "num_bins": 40, @@ -357,7 +357,7 @@ def compose(self) -> ComposeResult: # advanced settings, which most users can leave as the defaults with Collapsible( - title="Station slow-down & speed-up times", + title="Ship slow-down & speed-up times at waypoints", id="ship_advanced_collapsible", collapsed=True, ): diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index bdbc74e5..23cb015d 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -183,9 +183,9 @@ def stationkeeping_time(self, waypoint: Port | Waypoint) -> timedelta: ) return ( - self.ship_config.station_slowdown_time + self.ship_config.waypoint_slowdown_time + deployment_time - + self.ship_config.station_speedup_time + + self.ship_config.waypoint_speedup_time ) def _deployment_time( @@ -262,14 +262,14 @@ class ShipConfig(pydantic.BaseModel): ship_speed_knots: float = pydantic.Field(gt=0.0) - station_slowdown_time: timedelta = pydantic.Field( - serialization_alias="station_slowdown_time_minutes", - validation_alias="station_slowdown_time_minutes", + waypoint_slowdown_time: timedelta = pydantic.Field( + serialization_alias="waypoint_slowdown_time_minutes", + validation_alias="waypoint_slowdown_time_minutes", ge=timedelta(), ) - station_speedup_time: timedelta = pydantic.Field( - serialization_alias="station_speedup_time_minutes", - validation_alias="station_speedup_time_minutes", + waypoint_speedup_time: timedelta = pydantic.Field( + serialization_alias="waypoint_speedup_time_minutes", + validation_alias="waypoint_speedup_time_minutes", ge=timedelta(), ) @@ -277,12 +277,12 @@ class ShipConfig(pydantic.BaseModel): model_config = pydantic.ConfigDict(extra="forbid", populate_by_name=True) - @pydantic.field_serializer("station_slowdown_time", "station_speedup_time") + @pydantic.field_serializer("waypoint_slowdown_time", "waypoint_speedup_time") def _serialize_minutes(self, value: timedelta, _info) -> float: return value.total_seconds() / 60.0 @pydantic.field_validator( - "station_slowdown_time", "station_speedup_time", mode="before" + "waypoint_slowdown_time", "waypoint_speedup_time", mode="before" ) @classmethod def _validate_minutes(cls, value: int | float | timedelta) -> timedelta: diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index 46835505..7cf239d6 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -103,5 +103,5 @@ schedule: time: 1998-01-07 05:00:00 ship_config: ship_speed_knots: 10.0 - station_slowdown_time_minutes: 15.0 - station_speedup_time_minutes: 15.0 + waypoint_slowdown_time_minutes: 15.0 + waypoint_speedup_time_minutes: 15.0 diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 985c3bc6..efc0c32a 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -216,8 +216,8 @@ async def test_UI_changes(tmp_path): assert saved_wps[1].instrument == [InstrumentType.XBT] # the ship's slow-down / speed-up times survive a round trip through the UI, untouched - assert saved_expedition["ship_config"]["station_slowdown_time_minutes"] == 15.0 - assert saved_expedition["ship_config"]["station_speedup_time_minutes"] == 15.0 + assert saved_expedition["ship_config"]["waypoint_slowdown_time_minutes"] == 15.0 + assert saved_expedition["ship_config"]["waypoint_speedup_time_minutes"] == 15.0 # check verifiction methods are working by purposefully making invalid schedule (i.e. ship speed too slow to reach waypoints) invalid_speed = "0.0001" @@ -231,7 +231,7 @@ async def test_UI_changes(tmp_path): @pytest.mark.asyncio async def test_ship_decel_accel_editable_and_saved(tmp_path): - """The ship's station slow-down and speed-up times sit in their own collapsed section, and can be edited there and saved.""" + """The ship's waypoint slow-down and speed-up times sit in their own collapsed section, and can be edited there and saved.""" _make_expedition( tmp_path, [ @@ -263,17 +263,18 @@ async def test_ship_decel_accel_editable_and_saved(tmp_path): ) assert advanced.collapsed assert ( - expedition_editor.query_one("#station_slowdown_time", Input).value == "15.0" + expedition_editor.query_one("#waypoint_slowdown_time", Input).value + == "15.0" ) advanced.collapsed = False await pilot.pause() await simulate_input( - pilot, expedition_editor.query_one("#station_slowdown_time", Input), "20" + pilot, expedition_editor.query_one("#waypoint_slowdown_time", Input), "20" ) await simulate_input( - pilot, expedition_editor.query_one("#station_speedup_time", Input), "25" + pilot, expedition_editor.query_one("#waypoint_speedup_time", Input), "25" ) assert expedition_editor.has_unsaved_changes() @@ -290,8 +291,8 @@ async def test_ship_decel_accel_editable_and_saved(tmp_path): with open(tmp_path / EXPEDITION) as f: saved = yaml.safe_load(f) - assert saved["ship_config"]["station_slowdown_time_minutes"] == 20.0 - assert saved["ship_config"]["station_speedup_time_minutes"] == 25.0 + assert saved["ship_config"]["waypoint_slowdown_time_minutes"] == 20.0 + assert saved["ship_config"]["waypoint_speedup_time_minutes"] == 25.0 @pytest.mark.asyncio diff --git a/tests/expedition/expedition_dir/expedition.yaml b/tests/expedition/expedition_dir/expedition.yaml index c524a5bf..3d937764 100644 --- a/tests/expedition/expedition_dir/expedition.yaml +++ b/tests/expedition/expedition_dir/expedition.yaml @@ -51,5 +51,5 @@ instruments_config: period_minutes: 5.0 ship_config: ship_speed_knots: 10.0 - station_slowdown_time_minutes: 15.0 - station_speedup_time_minutes: 15.0 + waypoint_slowdown_time_minutes: 15.0 + waypoint_speedup_time_minutes: 15.0 diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index d7af2096..c9fc6c83 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -106,7 +106,7 @@ def test_stationkeeping_time_uses_slowest_deployment(expedition) -> None: / ctd_config.winch_speed_meter_per_second ) expected = ( - ship_config.station_slowdown_time + cast + ship_config.station_speedup_time + ship_config.waypoint_slowdown_time + cast + ship_config.waypoint_speedup_time ) assert expedition.stationkeeping_time(waypoint) == expected diff --git a/tests/expedition/test_simulate_schedule.py b/tests/expedition/test_simulate_schedule.py index d8505a93..8b2f1f53 100644 --- a/tests/expedition/test_simulate_schedule.py +++ b/tests/expedition/test_simulate_schedule.py @@ -44,8 +44,8 @@ def test_time_in_minutes_in_ship_schedule() -> None: assert instruments_config.ship_underwater_st_config.period == timedelta(minutes=5) ship_config = Expedition.from_yaml(TEST_EXPEDITION_YAML).ship_config - assert ship_config.station_slowdown_time == timedelta(minutes=15) - assert ship_config.station_speedup_time == timedelta(minutes=15) + assert ship_config.waypoint_slowdown_time == timedelta(minutes=15) + assert ship_config.waypoint_speedup_time == timedelta(minutes=15) def test_ship_path_inside_domain() -> None: diff --git a/tests/make_realistic/problems/test_simulator.py b/tests/make_realistic/problems/test_simulator.py index 802785c2..91bdbffc 100644 --- a/tests/make_realistic/problems/test_simulator.py +++ b/tests/make_realistic/problems/test_simulator.py @@ -59,8 +59,8 @@ def _make_simple_expedition( instruments = InstrumentsConfig() ship = ShipConfig( ship_speed_knots=10.0, - station_slowdown_time_minutes=15.0, - station_speedup_time_minutes=15.0, + waypoint_slowdown_time_minutes=15.0, + waypoint_speedup_time_minutes=15.0, ) return Expedition( schedule=schedule, instruments_config=instruments, ship_config=ship From 4f794b264e0e03f4a6fe6e99cdf95228a7a61fc2 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:49:10 +0200 Subject: [PATCH 28/39] new class attribute allowing specify whether ship needs to slow down for deployment, XBTs do not need to --- src/virtualship/instruments/argo_float.py | 3 +++ src/virtualship/instruments/base.py | 7 +++++++ src/virtualship/instruments/ctd.py | 3 +++ src/virtualship/instruments/drifter.py | 3 +++ src/virtualship/instruments/xbt.py | 3 +++ src/virtualship/models/expedition.py | 8 ++++++-- src/virtualship/utils.py | 5 +++++ tests/expedition/test_expedition.py | 16 ++++++++++++++++ tests/instruments/test_base.py | 20 ++++++++++++++++++++ tests/test_utils.py | 2 ++ 10 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/virtualship/instruments/argo_float.py b/src/virtualship/instruments/argo_float.py index b95d612b..7b8a5faf 100644 --- a/src/virtualship/instruments/argo_float.py +++ b/src/virtualship/instruments/argo_float.py @@ -231,6 +231,9 @@ def _format_log_metadata(ptcls_subset, mask, fieldset): class ArgoFloatInstrument(Instrument): """ArgoFloat instrument class.""" + # the ship has to stop for this instrument + requires_ship_to_stop: ClassVar[bool] = True + sensor_kernels: ClassVar[dict[SensorType, Callable]] = { SensorType.TEMPERATURE: _argo_sample_temperature, SensorType.SALINITY: _argo_sample_salinity, diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index d37597f1..ec5b950d 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -92,6 +92,10 @@ class Instrument(abc.ABC): # (required of every registered instrument; checked by @register_instrument) sensor_kernels: ClassVar[dict[SensorType, collections.abc.Callable]] + # all instruments declare whether the ship has to slow to a stop to deploy them (and so adds the waypoint slow-down and speed-up times) + # (required of every registered instrument; checked by @register_instrument) + requires_ship_to_stop: ClassVar[bool] + def __init__( self, expedition: Expedition, @@ -409,6 +413,9 @@ def __post_init__(self): class UnderwayInstrument(Instrument): """Intermediate base class for underway instruments, which perform variable sampling without ParticleSets.""" + # samples continuously while the ship is under way, so never needs it to stop + requires_ship_to_stop: ClassVar[bool] = False + @classmethod def deployment_time( cls, config: _InstrumentConfigMixin, seabed_depth: float | None diff --git a/src/virtualship/instruments/ctd.py b/src/virtualship/instruments/ctd.py index 1174d4a0..7699bcac 100644 --- a/src/virtualship/instruments/ctd.py +++ b/src/virtualship/instruments/ctd.py @@ -119,6 +119,9 @@ def _ctd_cast(particles, fieldset): class CTDInstrument(Instrument): """CTD instrument class.""" + # the ship has to stop for this instrument + requires_ship_to_stop: ClassVar[bool] = True + sensor_kernels: ClassVar[dict[SensorType, Callable]] = { SensorType.TEMPERATURE: _sample_temperature, SensorType.SALINITY: _sample_salinity, diff --git a/src/virtualship/instruments/drifter.py b/src/virtualship/instruments/drifter.py index 833f1c27..3fa780eb 100644 --- a/src/virtualship/instruments/drifter.py +++ b/src/virtualship/instruments/drifter.py @@ -69,6 +69,9 @@ def _check_lifetime(particles, fieldset): class DrifterInstrument(Instrument): """Drifter instrument class.""" + # the ship has to stop for this instrument + requires_ship_to_stop: ClassVar[bool] = True + sensor_kernels: ClassVar[dict[SensorType, Callable]] = { SensorType.TEMPERATURE: _sample_temperature, } diff --git a/src/virtualship/instruments/xbt.py b/src/virtualship/instruments/xbt.py index e878465d..dbf40be6 100644 --- a/src/virtualship/instruments/xbt.py +++ b/src/virtualship/instruments/xbt.py @@ -82,6 +82,9 @@ def _xbt_cast(particles, fieldset): class XBTInstrument(Instrument): """XBT instrument class.""" + # the ship can deploy this instrument whilst moving, so never needs to stop + requires_ship_to_stop: ClassVar[bool] = False + sensor_kernels: ClassVar[dict[SensorType, Callable]] = { SensorType.TEMPERATURE: _sample_temperature, } diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 23cb015d..ddad57c4 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -172,10 +172,14 @@ def stationkeeping_time(self, waypoint: Port | Waypoint) -> timedelta: Stationkeeping time = slow-down time + deployment time + speed-up time. Assumes insturments are deployed concurrently, so the longest deployment time is used. - Empty waypoints (no instruments) cost nothing/ship sails through without slowing. + The ship only slows down once, and only if at least one instrument needs it to stop. + Waypoints with no instruments, or only instruments which can be deployed without slowing (e.g. XBT), add no time at all. """ instruments = self._wp_instruments(waypoint) - if not instruments: + if not any( + get_instrument_class(instrument).requires_ship_to_stop + for instrument in instruments + ): return timedelta() deployment_time = max( diff --git a/src/virtualship/utils.py b/src/virtualship/utils.py index 12c41204..ab9674cc 100644 --- a/src/virtualship/utils.py +++ b/src/virtualship/utils.py @@ -141,6 +141,11 @@ def decorator(cls): f"Instrument '{cls.__name__}' must define 'sensor_kernels' as a class attribute." ) + if not isinstance(getattr(cls, "requires_ship_to_stop", None), bool): + raise TypeError( + f"Instrument '{cls.__name__}' must define 'requires_ship_to_stop' as a bool class attribute." + ) + INSTRUMENT_CLASS_MAP[instrument_type] = cls SUPPORTED_SENSORS_MAP[instrument_type] = frozenset(cls.sensor_kernels) return cls diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index c9fc6c83..81d73aab 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -146,6 +146,22 @@ def test_stationkeeping_time_single_instrument(expedition) -> None: ) +def test_stationkeeping_time_xbt_does_not_stop_ship(expedition) -> None: + """An XBT is launched from a moving ship, so alone it costs no stationkeeping time.""" + ship_config = expedition.ship_config + xbt_only = Waypoint(location=Location(0, 0), instrument=InstrumentType.XBT) + xbt_and_drifter = Waypoint( + location=Location(0, 0), + instrument=[InstrumentType.XBT, InstrumentType.DRIFTER], + ) + + assert expedition.stationkeeping_time(xbt_only) == timedelta(0) + # another instrument at the same waypoint still makes the ship stop, and the XBT adds nothing on top + assert expedition.stationkeeping_time(xbt_and_drifter) == ( + ship_config.waypoint_slowdown_time + ship_config.waypoint_speedup_time + ) + + def test_stationkeeping_time_no_instruments(expedition) -> None: """The ship doesn't slows down where nothing is deployed, therefore ports and bare waypoints cost nothing.""" assert expedition.stationkeeping_time( diff --git a/tests/instruments/test_base.py b/tests/instruments/test_base.py index 922630c6..c1eceb1a 100644 --- a/tests/instruments/test_base.py +++ b/tests/instruments/test_base.py @@ -153,6 +153,8 @@ class DummyInstrument(Instrument): sensor_kernels = {} # noqa + requires_ship_to_stop = False + @classmethod def deployment_time(cls, config, seabed_depth): """Dummy deployment time for test.""" @@ -420,6 +422,8 @@ def test_register_instrument_without_sensor_kernels_error(isolated_registry): @register_instrument(InstrumentType.CTD) class ErrorInstrument(Instrument): + requires_ship_to_stop = False + @classmethod def deployment_time(cls, config, seabed_depth): return timedelta() @@ -440,6 +444,22 @@ def simulate(self, measurements, out_path): pass +def test_register_instrument_without_requires_ship_to_stop_error(isolated_registry): + """Registering an instrument that leaves requires_ship_to_stop unimplemented raises TypeError.""" + with pytest.raises(TypeError, match="requires_ship_to_stop"): + + @register_instrument(InstrumentType.CTD) + class ErrorInstrument(Instrument): + sensor_kernels = {} # noqa + + @classmethod + def deployment_time(cls, config, seabed_depth): + return timedelta() + + def simulate(self, measurements, out_path): + pass + + def test_instrument_samples_initial_conditions(fieldset, pset): """_sample_initial adds initial conditions to particles.""" psetT_preinit = pset.temperature.copy() # before sampling initial conditions diff --git a/tests/test_utils.py b/tests/test_utils.py index 8181ab08..6dd3c053 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -39,6 +39,8 @@ def dummy_instrument(): class DummyInstrument(Instrument): sensor_kernels = {} # noqa + requires_ship_to_stop = False + @classmethod def deployment_time(cls, config, seabed_depth): return datetime.timedelta() From 15ade715b06e377c3268fe975e577352283e7ef6 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:53:42 +0200 Subject: [PATCH 29/39] make docstring more consistent with rest of codebase lingo --- src/virtualship/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/virtualship/utils.py b/src/virtualship/utils.py index ab9674cc..7a74d869 100644 --- a/src/virtualship/utils.py +++ b/src/virtualship/utils.py @@ -130,7 +130,7 @@ def register_instrument(instrument_type): - """Register a concrete instrument class, refusing if doesn't meet the full Instrument contract.""" + """Register an Instrument subclass (e.g. CTD, Argo Float etc.), refusing if doesn't meet the full Instrument parent class specifications.""" def decorator(cls): if inspect.isabstract(cls): From 801b4cfdb76b42290924c0e4e9ed07dfd091b331 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:27:19 +0200 Subject: [PATCH 30/39] add more docs info on times stationkeeping time per instrument --- .../assignments/Research_proposal_intro.ipynb | 29 ++++++++++--------- docs/user-guide/assignments/sail_the_ship.md | 4 +-- docs/user-guide/quickstart.md | 10 +++++-- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/docs/user-guide/assignments/Research_proposal_intro.ipynb b/docs/user-guide/assignments/Research_proposal_intro.ipynb index aac53036..cfaa7170 100644 --- a/docs/user-guide/assignments/Research_proposal_intro.ipynb +++ b/docs/user-guide/assignments/Research_proposal_intro.ipynb @@ -45,11 +45,11 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "### Underway Data \n", + "### Underway Temperature and Salinity measurements\n", "\n", "Underway data collection, also known as underway sampling, refers to the process of collecting oceanographic and environmental data while a research vessel is in motion. This method allows scientists to continuously gather data on various parameters such as sea surface temperature and salinity. The water inlet is located on the hull of the ship at approximately 2 meters under the surface.\n", "\n", - "You can collect underway data during your entire cruise or on specific transects and may assume it doesn’t take any time to start or stop taking measurements." + "You can collect underway data during your entire cruise. These measurements can be taken continuously without interrupting the ship's speed. It doesn’t take any time to start or stop the ship for these measurements." ] }, { @@ -60,7 +60,9 @@ "\n", "An Acoustic Doppler Current Profiler (ADCP) is an instrument used to measure the speed and direction of ocean currents at various depths. It operates based on the principle of Doppler shift, where sound waves emitted by the instrument are reflected off moving particles in the water, such as plankton or sediment, and the frequency shift of the returning waves is used to calculate the velocity of the water.\n", "\n", - "You can deploy the ADCP on transects and choose between a shallow profiler capable of providing information to a depth of 150 m every 4 meters (the 300kHz seaSeven), or a long-range profiler capable of providing about 1000m of range every 24 meters (the 38kHz Ocean Observer). You can collect ADCP data during your entire cruise or on specific transects and may assume it doesn’t take any time to start or end ADCP measurements.\n", + "You can choose between a shallow profiler capable of providing information to a depth of 150 m every 4 meters (the 300kHz seaSeven), or a long-range profiler capable of providing about 1000m of range every 24 meters (the 38kHz Ocean Observer). \n", + "\n", + "You can collect ADCP data during your entire cruise. These measurements can be taken continuously without interrupting the ship's speed. It doesn’t take any time to start or stop the ship for these measurements.\n", "\n", "" ] @@ -73,7 +75,7 @@ "\n", "An eXpendable BathyThermograph (XBT) is a probe that is dropped from a ship and measures the temperature as it falls through the water. The probe is designed to fall at a known rate, so that the depth of the temperature profile can be inferred from the time since it enters the water.\n", "\n", - "You may assume it doesn’t take any time to deploy XBTs.\n", + "You may assume it doesn’t take any time to deploy XBTs. They can be deployed while the ship is moving at full speed, so there is no time cost for slowing down or speeding up the ship.\n", "\n", "" ] @@ -87,15 +89,12 @@ "\n", "A Conductivity, Temperature, and Depth (CTD) sensor is an instrument used to measure the physical (or biogeochemical) properties of seawater in oceanographic and environmental research. It typically consists of sensors for conductivity (used to derive salinity), temperature, and pressure (used to derive depth). CTDs can also be equipped with additional sensors to measure biogeochemical parameters such as dissolved oxygen, chlorophyll, and nutrients. \n", "\n", - "Throughout a CTD deployment, the research vessels needs to stay in the same location. To calculate the time needed for a CTD deployment, take the following information into account:\n", - "\n", - "\n", - "* Add time to lower the vessels speed to zero and set the vessel to the appropriate direction against swell, current, wind, etc = 15 minutes \n", - "* Add time to deploy the CTD, and to recover = 10 minutes (2*5 min) \n", - "* Add time to raise the CTD down and up, usually at 1 m/s (winch capacity). We make an assumption in VirtualShip that the average CTD downcast/upcast takes 15 minutes.\n", - "* Add time to speed the ship back up again and set course to the next waypoint = 10 minutes\n", + "Throughout a CTD deployment, the research vessels needs to stay in the same location. To calculate the time needed for a CTD deployment at a given waypoint, take the following information into account:\n", "\n", - "In total, therefore, you can expect a CTD deployment to take approximately 50 minutes.\n", + "* The ship needs to slow down to a stop, which takes 15 minutes (default in VirtualShip)\n", + "* The CTD is lowered to the configured maximum depth or the sea floor, whichever is shallowest. The time taken therefore depends on the depth in that location and the winch speed (default is 1 m/s in VirtualShip). Remember that the time is doubled as well because the CTD needs to be brought back up.\n", + " * For example, if the CTD is lowered to 1000 m at a speed of 1 m/s, it will take approximately 16.7 minutes to reach that depth, and the same amount of time to raise it back up, so in total: approx. 33.4 minutes.\n", + "* The ship needs to speed back up again and set course to the next waypoint, which takes another 15 minutes (default in VirtualShip).\n", "\n", "" ] @@ -108,7 +107,9 @@ "\n", "A surface drifter is an oceanographic instrument used to study surface currents and ocean circulation patterns. These devices are designed to drift passively with the surface currents while transmitting data on their position and sea surface temperature.\n", "\n", - "You should assume it takes a combined 20 minutes to both slow down and speed the ship back up, in order to safely deploy drifters from the ship. Make sure to account for this time in your planning.\n", + "For deployment, the ship needs to slow down to a stop, which takes 15 minutes (default in VirtualShip). You can assume the actual release takes no time but then after the deployment the ship needs to speed back up again and set course to the next waypoint, which takes another 15 minutes (default in VirtualShip).\n", + "\n", + "For deployment, the ship needs to slow down to a stop, which takes 15 minutes (default in VirtualShip). You can assume the actual release takes no time but then after the deployment the ship needs to speed back up again and set course to the next waypoint, which takes another 15 minutes (default in VirtualShip).\n", "\n", "" ] @@ -123,7 +124,7 @@ "\n", "The floats periodically descend to a predetermined depth, typically around 1000 meters, and then ascend to the surface, measuring temperature and salinity profiles along the way.\n", "\n", - "You should assume it takes a combined 20 minutes to both slow down and speed the ship back up, in order to safely deploy Argo floats from the ship. Make sure to account for this time in your planning.\n", + "For deployment, the ship needs to slow down to a stop, which takes 15 minutes (default in VirtualShip). You can assume the actual release takes no time but then after the deployment the ship needs to speed back up again and set course to the next waypoint, which takes another 15 minutes (default in VirtualShip).\n", " \n", "" ] diff --git a/docs/user-guide/assignments/sail_the_ship.md b/docs/user-guide/assignments/sail_the_ship.md index 48aafe24..6bd82138 100644 --- a/docs/user-guide/assignments/sail_the_ship.md +++ b/docs/user-guide/assignments/sail_the_ship.md @@ -161,9 +161,9 @@ It is important to ensure that the timings for each station are realistic. There ```{important} Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected. Time at a station is made up of slowing the ship down (15 minutes by default), carrying out the deployments (which depends on which instruments you deploy), and getting back up to speed again (15 minutes by default). -Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters, Argo floats and XBTs are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. A CTD is different, it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default +Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters and Argo floats are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. CDTs and XBTs are different. A CTD takes longer because it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default. However, an XBT doesn't even need the ship to slow down, so it can be deployed while the ship is moving at full speed. -If verification tells you that you would arrive somewhere too late, your waypoints with CTDs are the first place to look. +**See [here](../assignments/Research_proposal_intro.ipynb) for more information on the times taken for each instrument deployment.** ``` ### Instrument selection diff --git a/docs/user-guide/quickstart.md b/docs/user-guide/quickstart.md index 111451f6..9e474d9e 100644 --- a/docs/user-guide/quickstart.md +++ b/docs/user-guide/quickstart.md @@ -57,9 +57,8 @@ The `--start-date` should be set to your intended departure date/time. MFP's est ``` ```{important} -MFP usually allows a default of **45 minutes** at each station, and that default is what determines the waypoint times written into your `expedition.yaml`. That may well be enough for your expedition, but it may not be, so be prepared to adjust the times once you have chosen your instruments. -Watch out in particular for **CTDs in deep water**. A CTD is winched all the way down and hauled back up, so the time it takes depends on the depth: a cast at a station in more than 2000 m of water takes over an hour on the winch alone, which does not fit inside a 45 minute allowance. VirtualShip will tell you if your schedule does not add up when you come to run it. +MFP usually allows a default of **45 minutes** stationkeeping time at each station, and that default is what determines the waypoint times written into your `expedition.yaml`. That may well be enough for your expedition, but it may not be, so be prepared to adjust the times once you have chosen your instruments and come to [adjusting the waypoint datetimes](#waypoint-datetimes). ``` This will create a folder/directory called `EXPEDITION_NAME` with a single file: `expedition.yaml` containing details on the ship and instrument configurations, as well as the expedition schedule based on the sampling site coordinates (and, if specified, ports of departure/arrival) from your MFP export. The `--from-mfp` flag indicates that the exported route will be used, and `--start-date` is required alongside it. @@ -128,6 +127,13 @@ Your Port of Departure and Port of Arrival (if set in MFP) appear as special way ```{note} It is important to ensure that the timings for each station are realistic. There must be enough time for the ship to travel to each site at a realistic speed (~ 10 knots). The expedition schedule (and the ship's configuration) will be automatically verified when you press _Save Changes_ in the planning tool. + +Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected (see [below](#instrument-selection)). Time at a station is made up of slowing the ship down (15 minutes by default), carrying out the deployments (which depends on which instruments you deploy), and getting back up to speed again (15 minutes by default). + +Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters and Argo floats are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. CDTs and XBTs are different. A CTD takes longer because it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default. However, an XBT doesn't even need the ship to slow down, so it can be deployed while the ship is moving at full speed. + +**See [here](../assignments/Research_proposal_intro.ipynb) for more information on the times taken for each instrument deployment.** + ``` ### Instrument selection From 4883e281d15c8082ef3dbe7354d5d8aa445afe68 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:51:35 +0200 Subject: [PATCH 31/39] typos and make differences between ctd and xbt timings clearer --- docs/user-guide/assignments/sail_the_ship.md | 2 +- docs/user-guide/quickstart.md | 2 +- docs/user-guide/tutorials/working_with_expedition_yaml.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user-guide/assignments/sail_the_ship.md b/docs/user-guide/assignments/sail_the_ship.md index 6bd82138..d5e0e25b 100644 --- a/docs/user-guide/assignments/sail_the_ship.md +++ b/docs/user-guide/assignments/sail_the_ship.md @@ -161,7 +161,7 @@ It is important to ensure that the timings for each station are realistic. There ```{important} Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected. Time at a station is made up of slowing the ship down (15 minutes by default), carrying out the deployments (which depends on which instruments you deploy), and getting back up to speed again (15 minutes by default). -Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters and Argo floats are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. CDTs and XBTs are different. A CTD takes longer because it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default. However, an XBT doesn't even need the ship to slow down, so it can be deployed while the ship is moving at full speed. +Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters and Argo floats are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. A CTD is different. It takes longer because it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default. An XBT is different in the opposite way. It doesn't even need the ship to slow down, so it can be deployed while the ship is moving at full speed. **See [here](../assignments/Research_proposal_intro.ipynb) for more information on the times taken for each instrument deployment.** ``` diff --git a/docs/user-guide/quickstart.md b/docs/user-guide/quickstart.md index 9e474d9e..948b3666 100644 --- a/docs/user-guide/quickstart.md +++ b/docs/user-guide/quickstart.md @@ -130,7 +130,7 @@ It is important to ensure that the timings for each station are realistic. There Remember that MFP's default of 45 minutes per station may not be enough once your instruments are selected (see [below](#instrument-selection)). Time at a station is made up of slowing the ship down (15 minutes by default), carrying out the deployments (which depends on which instruments you deploy), and getting back up to speed again (15 minutes by default). -Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters and Argo floats are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. CDTs and XBTs are different. A CTD takes longer because it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default. However, an XBT doesn't even need the ship to slow down, so it can be deployed while the ship is moving at full speed. +Deployments can take place side by side, so only the slowest one matters for the total stationkeeping time at each waypoint. Drifters and Argo floats are simply released into the water and left behind, so they have no time cost apart from slowing down and speeding up the ship. A CTD is different. It takes longer because it has to be lowered and brought back up, so the ship waits for the full round trip, and a cast at a deep station can easily take longer than the MFP default. An XBT is different in the opposite way. It doesn't even need the ship to slow down, so it can be deployed while the ship is moving at full speed. **See [here](../assignments/Research_proposal_intro.ipynb) for more information on the times taken for each instrument deployment.** diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index 689ce356..8e5d078d 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -160,4 +160,4 @@ This section contains setting related to the ship itself, specifically: - **Ship speed (`ship_speed_knots`)**: The speed of the ship in knots (nautical miles per hour; where 1 knot = 1.852 km/h). Note in most cases this should be left as the default value unless there is a specific reason to change it. -- **Waypoint slow-down and speed-up times (`waypoint_slowdown_time_minutes`, `waypoint_speedup_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy any instruments. +- **Waypoint slow-down and speed-up times (`waypoint_slowdown_time_minutes`, `waypoint_speedup_time_minutes`)**: How long the ship takes to slow to a stop before deployments begin at a waypoint, and to get back up to cruising speed afterwards, respectively. These are added once at every waypoint where you deploy instruments which require the ship to slow down. From 741e0408da1a743facb16861fc84b25996f3d1da Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:02:11 +0200 Subject: [PATCH 32/39] revert changes not relevant to this PR --- src/virtualship/models/expedition.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 3fb898cb..29700f1e 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -114,12 +114,12 @@ def verify_schedule( raise ScheduleError( f"Waypoint planning is not valid: would arrive too late at {self._describe_waypoint(wp_i, wp_next)}. " f"Location: {wp_next.location} Time: {wp_next.time}. " - f"Currently projected to arrive at: {arrival_time.replace(microsecond=0)}." + f"Currently projected to arrive at: {arrival_time}." "\n\nHint: adding instruments may increase the amount of time spent stationary at a waypoints. " "Have you ensured that your schedule includes sufficient time for taking measurements, e.g. CTD casts (in addition to the time it takes to sail between waypoints)?\n" ) - # mark this schedule as verified (so that subsequent stages of the workflow can proceed without re-verifying) + # finally, mark this schedule as verified (so that subsequent stages of the workflow can proceed without re-verifying) self.schedule._verified = True print("... All good to go!") From 857ff61c689fbc584946e7de32ddb01c41ea0017 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:04:27 +0200 Subject: [PATCH 33/39] clarify comment --- src/virtualship/cli/_initialise.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/virtualship/cli/_initialise.py b/src/virtualship/cli/_initialise.py index 8a58c618..5bd14d4a 100644 --- a/src/virtualship/cli/_initialise.py +++ b/src/virtualship/cli/_initialise.py @@ -133,7 +133,7 @@ def _mfp_to_yaml(file_path: Path, start_date: str, output_path: Path): has_latlon = lat is not None and lon is not None waypoints.append(Port(location=loc, time=time_val if has_latlon else None)) else: - # numbered as in the generated expedition.yaml's '# Waypoint N' comments + # waypoint numbers for warning messages, same as the generated expedition.yaml's 'Waypoint #N' order waypoint_number = sum(isinstance(wp, Waypoint) for wp in waypoints) + 1 sea_depth = _parse_mfp_sea_depth( row["Sea Depth"], f"Waypoint {waypoint_number}" From fd79829cef0f3584c06777341554d712149da693 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:05:08 +0200 Subject: [PATCH 34/39] revert to previous explanation of sea depth field, with a few enhancements --- docs/user-guide/tutorials/working_with_expedition_yaml.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index 0aae1a50..76a36477 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -117,9 +117,9 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint, as a negative number of metres (e.g. `-1500.0` for a site with a depth of 1500 m). This is most relevant to CTDs where their deployment time has a depth dependence. It can be left as `null` if you don't know the depth, in which case e.g. CTD casts are assumed to reach the CTD's `max_depth_meter`, giving the slowest (worst-case) cast time. It will be automatically filled in by `virtualship init` if you use the `--from-mfp` flag. +- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint. This value matters because it determines how a long a deployment takes if there is a depth dependence, e.g. for a CTD that has to be lowered and raised. If you leave it as `null`, VirtualShip doesn't know how deep the water is and assumes the CTD descends all the way to its configured `max_depth_meter`. That is the slowest possible cast, so your schedule is verified against a worst case rather than an optimistic one in the simulations. It will be automatically filled in by `virtualship init` if you use the `--from-mfp` flag. -```{note} +```{warning} Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. This is the opposite sign convention to the MFP export, which reports sea depth as a positive number. ``` From cdae68f91624b94ff36356da73bd7060ca181eed Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:07:21 +0200 Subject: [PATCH 35/39] minimise docstring --- src/virtualship/cli/_initialise.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/virtualship/cli/_initialise.py b/src/virtualship/cli/_initialise.py index 5bd14d4a..409e00e2 100644 --- a/src/virtualship/cli/_initialise.py +++ b/src/virtualship/cli/_initialise.py @@ -68,12 +68,7 @@ def _initialise( def _parse_mfp_sea_depth(value, waypoint_name: str) -> float | None: - """ - Sea depth from an MFP export, as a negative elevation in metres. - - MFP reports depth as a positive number of metres (usually with an "~"). - Returns None (i.e. unknown depth) for blank, unreadable, or non-positive values. - """ + """Sea depth from an MFP export, as a negative elevation in metres.""" if pd.isna(value): return None From 20210d2375ce4f1f0b39d30213db9ca0bd820089 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:25:01 +0200 Subject: [PATCH 36/39] fix duplication mistake --- docs/user-guide/assignments/Research_proposal_intro.ipynb | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/user-guide/assignments/Research_proposal_intro.ipynb b/docs/user-guide/assignments/Research_proposal_intro.ipynb index cfaa7170..52e74651 100644 --- a/docs/user-guide/assignments/Research_proposal_intro.ipynb +++ b/docs/user-guide/assignments/Research_proposal_intro.ipynb @@ -109,8 +109,6 @@ "\n", "For deployment, the ship needs to slow down to a stop, which takes 15 minutes (default in VirtualShip). You can assume the actual release takes no time but then after the deployment the ship needs to speed back up again and set course to the next waypoint, which takes another 15 minutes (default in VirtualShip).\n", "\n", - "For deployment, the ship needs to slow down to a stop, which takes 15 minutes (default in VirtualShip). You can assume the actual release takes no time but then after the deployment the ship needs to speed back up again and set course to the next waypoint, which takes another 15 minutes (default in VirtualShip).\n", - "\n", "" ] }, From bf92d29328335b29c6c62d2298785c70036d34ef Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:52:32 +0200 Subject: [PATCH 37/39] default to not having sea_depth_meter in expedition.yaml when not using --from-mfp --- .../tutorials/working_with_expedition_yaml.md | 2 +- src/virtualship/models/expedition.py | 3 ++ src/virtualship/static/expedition.yaml | 5 --- tests/expedition/test_expedition.py | 41 +++++++++++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index 76a36477..baac7960 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -117,7 +117,7 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint. This value matters because it determines how a long a deployment takes if there is a depth dependence, e.g. for a CTD that has to be lowered and raised. If you leave it as `null`, VirtualShip doesn't know how deep the water is and assumes the CTD descends all the way to its configured `max_depth_meter`. That is the slowest possible cast, so your schedule is verified against a worst case rather than an optimistic one in the simulations. It will be automatically filled in by `virtualship init` if you use the `--from-mfp` flag. +- **Sea depth (`sea_depth_meter`, optional)**: The estimated water depth at the waypoint, as a negative number of metres (e.g. `-1500.0`). It is filled in automatically by `virtualship init --from-mfp`, and can otherwise be added by hand. It is only used to estimate how long depth-dependent deployments (e.g. CTD casts) hold the ship at the waypoint. If it is left out, these are assumed to reach the instrument's configured maximum depth, so time taken at shallow-water waypoints could be overestimated if the configured max depth exceeds the actual depth. The simulated measurements are unaffected either way because the instruments will find and use the actual depth at simulation time. ```{warning} Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. This is the opposite sign convention to the MFP export, which reports sea depth as a positive number. diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 29700f1e..fdfd18db 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -250,6 +250,9 @@ def _annotate(self): else: waypoint_number += 1 annotated.append(f"{indent}# Waypoint {waypoint_number}\n") + # only written when known (e.g. from an MFP export); omitted means unknown + if wp_data.get("sea_depth_meter") is None: + wp_data.pop("sea_depth_meter", None) # dump the waypoint on its own so its block can be indented and inserted independently of the others wp_lines = yaml.dump([wp_data], default_flow_style=False).splitlines( diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index 7cf239d6..bf4b87f5 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -62,7 +62,6 @@ schedule: location: latitude: 0.01 longitude: 0.01 - sea_depth_meter: null time: 1998-01-02 00:00:00 # Waypoint 2 - instrument: @@ -71,7 +70,6 @@ schedule: location: latitude: 0.02 longitude: 0.02 - sea_depth_meter: null time: 1998-01-03 01:00:00 # Waypoint 3 - instrument: @@ -79,7 +77,6 @@ schedule: location: latitude: 0.03 longitude: 0.03 - sea_depth_meter: null time: 1998-01-04 02:00:00 # Waypoint 4 - instrument: @@ -87,14 +84,12 @@ schedule: location: latitude: 0.04 longitude: 0.04 - sea_depth_meter: null time: 1998-01-05 03:00:00 # Waypoint 5 - instrument: [] location: latitude: 0.05 longitude: 0.05 - sea_depth_meter: null time: 1998-01-06 04:00:00 # Port of Arrival - location: diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index 81d73aab..7d23e691 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -471,6 +471,47 @@ def test_annotate_no_duplicate_comments(base_expedition, waypoints) -> None: assert len(set(comments)) == len(comments) +def test_sea_depth_only_written_when_known(tmp_path, base_expedition) -> None: + """An unknown sea depth is left out of expedition.yaml entirely (rather than written as null), and a known one is kept.""" + known = Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1), + instrument=[InstrumentType.CTD], + sea_depth_meter=-1500.0, + ) + unknown = Waypoint( + location=Location(0, 0.01), + time=datetime(2022, 1, 2), + instrument=[InstrumentType.CTD], + ) + expedition = _with_schedule( + base_expedition, + Schedule( + waypoints=[ + Port(location=None, time=None), + known, + unknown, + Port(location=None, time=None), + ] + ), + ) + + out_path = tmp_path / EXPEDITION + expedition.to_yaml(out_path) + text = out_path.read_text() + + assert text.count("sea_depth_meter") == 1 + assert "sea_depth_meter: -1500.0" in text + # ports' null location/time are still written, so placeholders remain visible + assert "location: null" in text + + reloaded = Expedition.from_yaml(out_path) + assert [wp.sea_depth_meter for wp in reloaded.schedule.waypoints[1:3]] == [ + -1500.0, + None, + ] + + def test_wps_in_use(base_expedition): """Test that _get_wps_in_use() correctly returns waypoints excluding placeholder ports.""" base_time = datetime.strptime("1950-01-01", "%Y-%m-%d") From d06d9e6a0996c264f428f8a332a6b5221ef6d23d Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:06:59 +0200 Subject: [PATCH 38/39] "left out" -> "left empty" --- docs/user-guide/tutorials/working_with_expedition_yaml.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index baac7960..56d37506 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -117,7 +117,7 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -- **Sea depth (`sea_depth_meter`, optional)**: The estimated water depth at the waypoint, as a negative number of metres (e.g. `-1500.0`). It is filled in automatically by `virtualship init --from-mfp`, and can otherwise be added by hand. It is only used to estimate how long depth-dependent deployments (e.g. CTD casts) hold the ship at the waypoint. If it is left out, these are assumed to reach the instrument's configured maximum depth, so time taken at shallow-water waypoints could be overestimated if the configured max depth exceeds the actual depth. The simulated measurements are unaffected either way because the instruments will find and use the actual depth at simulation time. +- **Sea depth (`sea_depth_meter`, optional)**: The estimated water depth at the waypoint, as a negative number of metres (e.g. `-1500.0`). It is filled in automatically by `virtualship init --from-mfp`, and can otherwise be added by hand. It is only used to estimate how long depth-dependent deployments (e.g. CTD casts) hold the ship at the waypoint. If it is left empty, these are assumed to reach the instrument's configured maximum depth, so time taken at shallow-water waypoints could be overestimated if the configured max depth exceeds the actual depth. The simulated measurements are unaffected either way because the instruments will find and use the actual depth at simulation time. ```{warning} Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. This is the opposite sign convention to the MFP export, which reports sea depth as a positive number. From 2e7618349f0feab0e4153c14e4ee0dfd8bbe6c39 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:23:51 +0200 Subject: [PATCH 39/39] avoid redundant None when popping --- src/virtualship/models/expedition.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index fdfd18db..18ee986e 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -250,9 +250,10 @@ def _annotate(self): else: waypoint_number += 1 annotated.append(f"{indent}# Waypoint {waypoint_number}\n") + # only written when known (e.g. from an MFP export); omitted means unknown - if wp_data.get("sea_depth_meter") is None: - wp_data.pop("sea_depth_meter", None) + if wp_data["sea_depth_meter"] is None: + wp_data.pop("sea_depth_meter") # dump the waypoint on its own so its block can be indented and inserted independently of the others wp_lines = yaml.dump([wp_data], default_flow_style=False).splitlines(