diff --git a/pyproject.toml b/pyproject.toml index a5f1f8e6b..0b2fa3595 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,6 +125,7 @@ warn_unused_ignores = true exclude = [ # Temporarily exclude files. As we improve typechecking across the codebase, remove these + "src/virtualship/cli/_initialise.py", "src/virtualship/cli/_plan.py", "src/virtualship/cli/_run.py", "src/virtualship/cli/commands.py", diff --git a/src/virtualship/cli/_initialise.py b/src/virtualship/cli/_initialise.py new file mode 100644 index 000000000..d5cfbc4b6 --- /dev/null +++ b/src/virtualship/cli/_initialise.py @@ -0,0 +1,260 @@ +import os +import re +import warnings +from datetime import timedelta +from pathlib import Path + +import click +import pandas as pd +import yaml + +from virtualship.models import ( + Expedition, + InstrumentsConfig, + Location, + Port, + Schedule, + Waypoint, +) +from virtualship.utils import EXPEDITION, _get_example_expedition + +ERR_SUPPLEMENT = "If the MFP export format has changed, please submit an issue at: https://github.com/Parcels-code/virtualship/issues." + + +def _initialise( + path: str | Path, from_mfp: str | None = None, start_date: str | None = None +): + path = Path(path) + path.mkdir(exist_ok=True) + + expedition = path / EXPEDITION + + if expedition.exists(): + raise FileExistsError( + f"File '{expedition}' already exists. Please remove it or choose another directory." + ) + + if from_mfp: + mfp_file = Path(from_mfp) + click.echo(f"Generating schedule from {mfp_file}...") + + # catch warnings raised to propagate them via click.echo + with warnings.catch_warnings(record=True) as captured_warnings: + warnings.simplefilter("always") + _mfp_to_yaml(mfp_file, start_date, expedition) + + indent = " " * 4 + click.echo( + "\n⚠️ The generated schedule does not contain INSTRUMENT selections. ⚠️" + "\n\nNow please either use the `\033[4mvirtualship plan\033[0m` app to complete the configuration, " + "\nOR edit 'expedition.yaml' and manually add the instrument selections under the 'schedule' heading." + "\n\nIf editing 'expedition.yaml' manually:" + "\n\n🌡️ Expected instrument(s) format: one line per instrument e.g." + f"\n\n{indent * 4}waypoints:\n{indent * 4}- instrument:\n{indent * 5}- CTD\n{indent * 5}- ARGO_FLOAT\n" + ) + + # output captured warnings to the terminal + if captured_warnings: + click.echo("\n❗️ WARNINGS:") + for w in captured_warnings: + click.echo(f"{indent}• {w.message}") + click.echo( + f"\n{indent}If you believe any of these warnings are incorrect (e.g. you have selected departure/arrival ports), and {ERR_SUPPLEMENT.replace('If ', '')}\n" + ) + else: + expedition.write_text(_get_example_expedition()) + + click.echo(f"Created '{expedition.name}' at {path}.") + + +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) + + # convert start_date string to datetime object if needed, ensuring it's standard Python datetime + if isinstance(start_date, str): + current_time = pd.to_datetime(start_date).to_pydatetime() + elif isinstance(start_date, pd.Timestamp): + current_time = start_date.to_pydatetime() + else: + current_time = start_date + + waypoints = [] + previous_timedelta = None + + for i, row in mfp_data.iterrows(): + if i > 0: + current_time += previous_timedelta + + is_port = "Port" in str(row["Station"]) or "Port" in str(row["Type"]) + lat = None if pd.isna(row["Latitude"]) else float(row["Latitude"]) + lon = None if pd.isna(row["Longitude"]) else float(row["Longitude"]) + loc = Location(latitude=lat, longitude=lon) + + # Ensure timestamp passed is a native python datetime (or string) to prevent PyYAML pandas pickle tags + time_val = ( + current_time.to_pydatetime() + if isinstance(current_time, pd.Timestamp) + else current_time + ) + + if is_port: + 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)) + + previous_timedelta = ( + row["Total Time"] if pd.notna(row["Total Time"]) else timedelta(0) + ) + + # build and dump expedition YAML + static_yaml = yaml.safe_load(_get_example_expedition()) + expedition = Expedition( + schedule=Schedule(waypoints=waypoints), + instruments_config=InstrumentsConfig.model_validate( + static_yaml.get("instruments_config") + ), + ship_config=static_yaml.get("ship_config"), + ) + expedition.to_yaml(output_path) + + +def _validate_mfp_data(file_path: Path) -> pd.DataFrame: + """Load and validate MFP CruiseData export.""" + mfp_data = _load_mfp_export(file_path) + + # clean up column names + mfp_data.columns = mfp_data.columns.astype(str).str.strip() + junk_col_pattern = r"^(Unnamed:.*||\.\d+)$" + mfp_data = mfp_data.loc[:, ~mfp_data.columns.str.match(junk_col_pattern)] + + expected_columns = [ + "Station", + "Type", + "Latitude", + "Longitude", + "Sea Depth", + "Time at Station", + "Travel Time to Next", + "Distance to Next (NM)", + "Ship Speed (kn)", + "EEZ", + ] + expected_set = set(expected_columns) + actual_set = set(mfp_data.columns) + + missing_columns = expected_set - actual_set + if missing_columns: + raise ValueError( + f"Error: Found columns {list(actual_set)}, but expected columns {list(expected_columns)}. " + f"Are you sure that you're using the correct export from MFP?\n\n{ERR_SUPPLEMENT}" + ) + + extra_columns = actual_set - expected_set + if extra_columns: + warnings.warn( + f"Found additional unexpected columns {list(extra_columns)}. Manually added columns have no effect.", + stacklevel=2, + ) + + # safe float conversion for lat/lon + for coord in ["Latitude", "Longitude"]: + if mfp_data[coord].dtype in ["object", "string"]: + mfp_data[coord] = pd.to_numeric( + mfp_data[coord].astype(str).str.replace(",", "."), errors="coerce" + ) + + # check for missing departure/arrival ports and add placeholders if necessary + # check against both 'Station' and 'Type' columns; variations can occur when importing to MFP before re-exporting + has_departure = ( + "Departure Port" in mfp_data["Station"].values + or "Departure Port" in mfp_data["Type"].values + ) + has_arrival = ( + "Arrival Port" in mfp_data["Station"].values + or "Arrival Port" in mfp_data["Type"].values + ) + + if not has_departure or not has_arrival: + warnings.warn( + "The MFP export is missing either a 'Departure Port' or 'Arrival Port', or both. " + "Any missing port will be replaced with an empty placeholder in `expedition.yaml` but will be ignored in the simulation. " + "If missing the 'Departure Port', the prescribed start date will be used for Waypoint #1 instead. ", + stacklevel=2, + ) + + if not has_departure: + dept_row = _create_port_row(expected_columns, "Departure Port") + mfp_data = pd.concat([dept_row, mfp_data], ignore_index=True) # first row + + if not has_arrival: + arr_row = _create_port_row(expected_columns, "Arrival Port") + mfp_data = pd.concat([mfp_data, arr_row], ignore_index=True) # last row + + # Drop unexpected columns + mfp_data = mfp_data[list(expected_columns)] + + # convert 'Travel Time to Next' and 'Time at Station' to timedelta + mfp_data["Travel Time to Next"] = mfp_data["Travel Time to Next"].apply( + _mfp_string_to_timedelta + ) + mfp_data["Time at Station"] = mfp_data["Time at Station"].apply( + _mfp_string_to_timedelta + ) + + # combine 'Travel Time to Next' and 'Time at Station' into a single 'Total Time' column + # add 0 when Time at Station is NaN, to avoid NaT in Total Time, but not to Travel Time to keep NaT at the arrival port + mfp_data["Total Time"] = mfp_data["Travel Time to Next"] + mfp_data[ + "Time at Station" + ].fillna(pd.Timedelta(0)) + + return mfp_data + + +def _load_mfp_export(file_path: Path) -> pd.DataFrame: + if not os.path.isfile(file_path): + raise FileNotFoundError(f"File not found: {file_path}") + + try: + return pd.read_excel(file_path).dropna(how="all", axis=1) # drop empty columns + except Exception as e: + raise RuntimeError( + "Could not read coordinates data from the provided file. " + "Ensure it is an exported .xlsx file from MFP." + ) from e + + +def _create_port_row(columns, port_type: str) -> pd.DataFrame: + """Generate a single placeholder row for missing departure/arrival ports.""" + row = {col: None for col in columns} + row["Station"] = port_type + row["Type"] = port_type + return pd.DataFrame([row]) + + +def _mfp_string_to_timedelta(value: str | None) -> timedelta | None: + """Parse MFP duration string (e.g., '0d 13h 13m') to timedelta.""" + if pd.isna(value): + return None + + match = re.search(r"(\d+)d\s*(\d+)h\s*(\d+)m", str(value)) + if match: + days, hours, minutes = map(int, match.groups()) + return timedelta(days=days, hours=hours, minutes=minutes) + + else: + raise ValueError( + f"Invalid MFP duration format: '{value}'. Expected format: 'Xd Yh Zm' (e.g., '0d 13h 13m'). {ERR_SUPPLEMENT}" + ) + + +def _validate_start_date(ctx, param, value): + """Enforce --start-date when --from-mfp is used.""" + if ctx.params.get("from_mfp"): + if not value: + raise click.BadParameter( + "The '--start-date' option is required when using '--from-mfp'." + "\n\nExpected format: 'YYYY-MM-DD HH:MM:SS' (with quotes, e.g., '2023-10-20 01:00:00'). If only the date is provided, the time will default to 00:00:00." + ) + return value diff --git a/src/virtualship/cli/_plan.py b/src/virtualship/cli/_plan.py index 9446fb6ff..1acbd97da 100644 --- a/src/virtualship/cli/_plan.py +++ b/src/virtualship/cli/_plan.py @@ -7,7 +7,6 @@ from textual.app import App, ComposeResult from textual.containers import Container, Horizontal, VerticalScroll from textual.dom import NoMatches -from textual.markup import escape from textual.screen import ModalScreen, Screen from textual.validation import Function, Integer from textual.widgets import ( @@ -43,7 +42,8 @@ Waypoint, XBTConfig, ) -from virtualship.utils import EXPEDITION, _get_waypoint_latlons +from virtualship.models.expedition import Port +from virtualship.utils import EXPEDITION, INCOMPLETE_PORT_MSG UNEXPECTED_MSG_ONSAVE = ( "Please ensure that:\n" @@ -86,6 +86,14 @@ def _default_sensors(config_class) -> list: return sensors_field.default_factory() +def parse_waypoint_datetime(year, month, day, hour, minute): + """Parses date/time values into a datetime object if all components are present.""" + values = (year, month, day, hour, minute) + if all(v is not None and v != Select.NULL for v in values): + return datetime.datetime(*(int(v) for v in values)) + return None + + DEFAULT_TS_CONFIG = {"period_minutes": 5.0} DEFAULT_ADCP_CONFIG = { @@ -434,8 +442,8 @@ def save_changes(self) -> bool: """Save changes to expedition.yaml.""" try: self._update_ship_speed() - self._update_instrument_configs() self._update_schedule() + self._update_instrument_configs() self.expedition.to_yaml(self.path.joinpath(EXPEDITION)) return True except UserError: @@ -496,6 +504,7 @@ def _update_instrument_configs(self): kwargs["max_depth_meter"] = -1000.0 else: kwargs["max_depth_meter"] = -150.0 + # collect sensor toggles default_sensor_configs = _default_sensors(config_class) if default_sensor_configs: @@ -506,20 +515,22 @@ def _update_instrument_configs(self): f"#{instrument_name}_sensor_{sc.sensor_type.value}", Switch ).value ] - if not sensors: - # for schedule-based instruments, only raise if actually used in a waypoint - # for underway, this is handled by the on/off toggle - instrument_type = info.get("instrument_type") - is_active = instrument_type is None or any( - instrument_type - in ( - wp.instrument - if isinstance(wp.instrument, list) - else [wp.instrument] - ) - for wp in self.expedition.schedule.waypoints - if wp.instrument + + instrument_type = info.get("instrument_type") + + # safe check for instrument existence across all waypoint types + is_active = instrument_type is None or any( + instrument_type + in ( + wp.instrument + if isinstance(wp.instrument, list) + else [wp.instrument] ) + for wp in self.expedition.schedule.waypoints + if getattr(wp, "instrument", None) + ) + + if not sensors: if is_active: title = info.get( "title", instrument_name.replace("_", " ").title() @@ -528,9 +539,16 @@ def _update_instrument_configs(self): f"'{title}' has no sensors selected. " f"At least one sensor must be enabled for each active instrument." ) - kwargs["sensors"] = ( - sensors if sensors else _default_sensors(config_class) - ) + else: + # if the instrument is not active in the schedule and no sensors are selected: + # reset to default sensors (or keep default_sensor_configs) so pydantic validation passes. + sensors = [ + SensorConfig(sensor_type=sc.sensor_type) + for sc in default_sensor_configs + ] + + kwargs["sensors"] = sensors + try: setattr( self.expedition.instruments_config, @@ -553,28 +571,41 @@ def _update_instrument_configs(self): def _update_schedule(self): for i, wp in enumerate(self.expedition.schedule.waypoints): - wp.location = Location( - latitude=float(self.query_one(f"#wp{i}_lat").value), - longitude=float(self.query_one(f"#wp{i}_lon").value), + wp.time = parse_waypoint_datetime( + self.query_one(f"#wp{i}_year", Select).value, + self.query_one(f"#wp{i}_month", Select).value, + self.query_one(f"#wp{i}_day", Select).value, + self.query_one(f"#wp{i}_hour", Select).value, + self.query_one(f"#wp{i}_minute", Select).value, ) - wp.time = datetime.datetime( - int(self.query_one(f"#wp{i}_year").value), - int(self.query_one(f"#wp{i}_month").value), - int(self.query_one(f"#wp{i}_day").value), - int(self.query_one(f"#wp{i}_hour").value), - int(self.query_one(f"#wp{i}_minute").value), - 0, - ) - wp.instrument = [] - for instrument in [inst for inst in InstrumentType if not inst.is_underway]: - switch_on = self.query_one(f"#wp{i}_{instrument.value}").value - if instrument.value == "DRIFTER" and switch_on: - count_str = self.query_one(f"#wp{i}_drifter_count").value - count = int(count_str) - assert count > 0 - wp.instrument.extend([InstrumentType.DRIFTER] * count) - elif switch_on: - wp.instrument.append(instrument) + + lat_val = self.query_one(f"#wp{i}_lat").value + lon_val = self.query_one(f"#wp{i}_lon").value + + if isinstance(wp, Port) and (lat_val == "" or lon_val == ""): + wp.location = Location( + latitude=float(lat_val) if lat_val != "" else None, + longitude=float(lon_val) if lon_val != "" else None, + ) + else: + wp.location = Location( + latitude=float(lat_val), + longitude=float(lon_val), + ) + + if not isinstance(wp, Port): + wp.instrument = [] + for instrument in [ + inst for inst in InstrumentType if not inst.is_underway + ]: + switch_on = self.query_one(f"#wp{i}_{instrument.value}").value + if instrument.value == "DRIFTER" and switch_on: + count_str = self.query_one(f"#wp{i}_drifter_count").value + count = int(count_str) + assert count > 0 + wp.instrument.extend([InstrumentType.DRIFTER] * count) + elif switch_on: + wp.instrument.append(instrument) @on(Input.Changed) def show_invalid_reasons(self, event: Input.Changed) -> None: @@ -613,10 +644,12 @@ def show_invalid_reasons(self, event: Input.Changed) -> None: @on(Button.Pressed, "#add_waypoint") def add_waypoint(self) -> None: - """Add a new waypoint to the schedule. Copies time from last waypoint if possible (Lat/lon and instruments blank).""" + """Add a new waypoint to the schedule (N.B. ports always remain). Copies time from last waypoint if possible (Lat/lon and instruments blank).""" try: - if self.expedition.schedule.waypoints: - last_wp = self.expedition.schedule.waypoints[-1] + wps = self.expedition.schedule.waypoints + if wps: + non_port_wps = [wp for wp in wps if not isinstance(wp, Port)] + last_wp = non_port_wps[-1] new_time = last_wp.time if last_wp.time else None new_wp = Waypoint( location=Location( @@ -632,7 +665,12 @@ def add_waypoint(self) -> None: time=None, instrument=[], ) - self.expedition.schedule.waypoints.append(new_wp) + + # add waypoint before the last port (arrival port) if it exists, otherwise at the end + insert_index = next( + (i for i, wp in reversed(list(enumerate(wps))) if isinstance(wp, Port)) + ) # just before arrival port + self.expedition.schedule.waypoints.insert(insert_index, new_wp) self.refresh_waypoint_widgets() except Exception as e: @@ -640,10 +678,18 @@ def add_waypoint(self) -> None: @on(Button.Pressed, "#remove_waypoint") def remove_waypoint(self) -> None: - """Remove the last waypoint from the schedule.""" + """Remove the last waypoint (non-port) from the schedule.""" try: - if self.expedition.schedule.waypoints: - self.expedition.schedule.waypoints.pop() + wps = self.expedition.schedule.waypoints + if wps: + last_wp_index = next( + ( + i + for i, wp in reversed(list(enumerate(wps))) + if isinstance(wp, Waypoint) + ) + ) + self.expedition.schedule.waypoints.pop(last_wp_index) self.refresh_waypoint_widgets() else: self.notify("No waypoints to remove.", severity="error", timeout=5) @@ -741,6 +787,15 @@ def shallow_changed(self, event: Switch.Changed) -> None: deep = self.query_one("#adcp_deep", Switch) deep.value = False + @on(Button.Pressed, "#info_button") + def info_pressed(self) -> None: + self.notify( + "[b]SeaSeven[/b]:\nShallow ADCP profiler capable of providing information to a depth of 150 m every 4 meters (300kHz)" + "\n\n[b]OceanObserver[/b]:\nLong-range ADCP profiler capable of providing ~ 1000m of depth range every 24 meters (38kHz)", + severity="warning", + timeout=20, + ) + class WaypointWidget(Static): def __init__(self, waypoint: Waypoint, index: int): @@ -748,150 +803,146 @@ def __init__(self, waypoint: Waypoint, index: int): self.waypoint = waypoint self.index = index + def _get_coord_value(self, coord: float | None) -> str: + """Return coordinate as string or empty string if None.""" + return str(coord) if coord is not None else "" + + def _get_minute_options(self) -> list[tuple[str, int]]: + """Generate minute options, inserting current minute if non-multiple of 5.""" + options = {(f"{m:02d}", m) for m in range(0, 60, 5)} + if self.waypoint.time and self.waypoint.time.minute % 5 != 0: + m = self.waypoint.time.minute + options.add((f"{m:02d}", m)) + return sorted(list(options), key=lambda x: x[1]) + + def _yield_coordinate_input( + self, label: str, field: str, validator_fn, placeholder: str, wp_id: int + ) -> ComposeResult: + """Yields a labeled coordinate input with its validation error label.""" + val = getattr(self.waypoint.location, field, None) + + yield Label(f" {label}:") + yield Input( + id=f"wp{wp_id}_{field}", + value=self._get_coord_value(val), + validators=[ + Function( + validator_fn, + f"INVALID: value must be {validator_fn.__doc__.lower()}", + ) + ], + type="number", + placeholder=placeholder, + classes=f"{field}itude-input", + ) + yield Label( + "", + id=f"validation-failure-label-wp{wp_id}_{field}", + classes="-hidden validation-failure", + ) + + def _yield_time_selectors(self, wp_id: int) -> ComposeResult: + """Yields year, month, day, hour, and minute Select controls.""" + time = self.waypoint.time + + yield Label("Year:") + yield Select( + [(str(y), y) for y in range(1993, datetime.datetime.now().year + 1)], + id=f"wp{wp_id}_year", + value=time.year if time else Select.NULL, + prompt="YYYY", + classes="year-select", + ) + yield Label("Month:") + yield Select( + [(f"{m:02d}", m) for m in range(1, 13)], + id=f"wp{wp_id}_month", + value=time.month if time else Select.NULL, + prompt="MM", + classes="month-select", + ) + yield Label("Day:") + yield Select( + [(f"{d:02d}", d) for d in range(1, 32)], + id=f"wp{wp_id}_day", + value=time.day if time else Select.NULL, + prompt="DD", + classes="day-select", + ) + yield Label("Hour:") + yield Select( + [(f"{h:02d}", h) for h in range(24)], + id=f"wp{wp_id}_hour", + value=time.hour if time else Select.NULL, + prompt="hh", + classes="hour-select", + ) + yield Label("Min:") + yield Select( + self._get_minute_options(), + id=f"wp{wp_id}_minute", + value=time.minute if time else Select.NULL, + prompt="mm", + classes="minute-select", + ) + + def _yield_instrument_controls(self, wp_id: int) -> ComposeResult: + """Yields instrument controls if waypoint is not a Port.""" + yield Label("Instruments:") + + for instrument in [i for i in InstrumentType if not i.is_underway]: + is_selected = instrument in (self.waypoint.instrument or []) + with Horizontal(): + yield Label(instrument.value) + # Matches expected #inst_ prefix or wp-indexed switch ID + yield Switch( + value=is_selected, + id=f"wp{wp_id}_{instrument.value}", + ) + + if instrument.value == "DRIFTER": + yield Label("Count") + yield Input( + id=f"wp{wp_id}_drifter_count", + value=str(self.get_drifter_count() if is_selected else ""), + type="integer", + placeholder="# of drifters", + validators=Integer( + minimum=1, + failure_description="INVALID: value must be > 0", + ), + classes="drifter-count-input", + ) + yield Label( + "", + id=f"validation-failure-label-wp{wp_id}_drifter_count", + classes="-hidden validation-failure", + ) + def compose(self) -> ComposeResult: try: with Collapsible( - title=f"[b]Waypoint {self.index + 1}[/b]", - collapsed=True, - id=f"wp{self.index + 1}", + title=self.get_title(), collapsed=True, id=f"wp{self.index}" ): if self.index > 0: yield Button( - "Copy Time & Instruments from Previous", + self.get_copy_button_text(), id=f"wp{self.index}_copy", variant="warning", ) - yield Label("Location:") - yield Label(" Latitude:") - yield Input( - id=f"wp{self.index}_lat", - value=str(self.waypoint.location.lat) - if self.waypoint.location.lat - is not None # is not None to handle if lat is 0.0 - else "", - validators=[ - Function( - is_valid_lat, - f"INVALID: value must be {is_valid_lat.__doc__.lower()}", - ) - ], - type="number", - placeholder="°N", - classes="latitude-input", - ) - yield Label( - "", - id=f"validation-failure-label-wp{self.index}_lat", - classes="-hidden validation-failure", - ) - yield Label(" Longitude:") - yield Input( - id=f"wp{self.index}_lon", - value=str(self.waypoint.location.lon) - if self.waypoint.location.lon - is not None # is not None to handle if lon is 0.0 - else "", - validators=[ - Function( - is_valid_lon, - f"INVALID: value must be {is_valid_lon.__doc__.lower()}", - ) - ], - type="number", - placeholder="°E", - classes="longitude-input", + yield Label("Location:") + yield from self._yield_coordinate_input( + "Latitude", "lat", is_valid_lat, "°N", self.index ) - yield Label( - "", - id=f"validation-failure-label-wp{self.index}_lon", - classes="-hidden validation-failure", + yield from self._yield_coordinate_input( + "Longitude", "lon", is_valid_lon, "°E", self.index ) yield Label("Time:") with Horizontal(): - yield Label("Year:") - yield Select( - [ - (str(year), year) - for year in range( - 1993, - datetime.datetime.now().year + 1, - ) - ], - id=f"wp{self.index}_year", - value=int(self.waypoint.time.year) - if self.waypoint.time - else Select.NULL, - prompt="YYYY", - classes="year-select", - ) - yield Label("Month:") - yield Select( - [(f"{m:02d}", m) for m in range(1, 13)], - id=f"wp{self.index}_month", - value=int(self.waypoint.time.month) - if self.waypoint.time - else Select.NULL, - prompt="MM", - classes="month-select", - ) - yield Label("Day:") - yield Select( - [(f"{d:02d}", d) for d in range(1, 32)], - id=f"wp{self.index}_day", - value=int(self.waypoint.time.day) - if self.waypoint.time - else Select.NULL, - prompt="DD", - classes="day-select", - ) - yield Label("Hour:") - yield Select( - [(f"{h:02d}", h) for h in range(24)], - id=f"wp{self.index}_hour", - value=int(self.waypoint.time.hour) - if self.waypoint.time - else Select.NULL, - prompt="hh", - classes="hour-select", - ) - yield Label("Min:") - minute_options = [(f"{m:02d}", m) for m in range(0, 60, 5)] - minute_value = ( - int(self.waypoint.time.minute) - if self.waypoint.time - else Select.NULL - ) - - # if the current minute is not a multiple of 5, add it to the options - if ( - self.waypoint.time - and self.waypoint.time.minute % 5 != 0 - and ( - f"{self.waypoint.time.minute:02d}", - self.waypoint.time.minute, - ) - not in minute_options - ): - minute_options = [ - ( - f"{self.waypoint.time.minute:02d}", - self.waypoint.time.minute, - ) - ] + minute_options - - minute_options = sorted(minute_options, key=lambda x: x[1]) + yield from self._yield_time_selectors(self.index) - yield Select( - minute_options, - id=f"wp{self.index}_minute", - value=minute_value, - prompt="mm", - classes="minute-select", - ) - - # fmt: off yield Horizontal( Button("+1 day", id="plus_one_day", variant="primary"), Button("+1 hour", id="plus_one_hour", variant="primary"), @@ -901,47 +952,32 @@ def compose(self) -> ComposeResult: Button("-30 minutes", id="minus_thirty_minutes", variant="default"), classes="time-adjust-buttons", ) - # fmt: on - - yield Label("Instruments:") - for instrument in [i for i in InstrumentType if not i.is_underway]: - is_selected = instrument in (self.waypoint.instrument or []) - with Horizontal(): - yield Label(instrument.value) - yield Switch( - value=is_selected, id=f"wp{self.index}_{instrument.value}" - ) - - if instrument.value == "DRIFTER": - yield Label("Count") - yield Input( - id=f"wp{self.index}_drifter_count", - value=str( - self.get_drifter_count() if is_selected else "" - ), - type="integer", - placeholder="# of drifters", - validators=Integer( - minimum=1, - failure_description="INVALID: value must be > 0", - ), - classes="drifter-count-input", - ) - yield Label( - "", - id=f"validation-failure-label-wp{self.index}_drifter_count", - classes="-hidden validation-failure", - ) - yield Horizontal( - Button( - "Remove Waypoint", id=f"wp{self.index}_remove", variant="error" + if not isinstance(self.waypoint, Port): + yield from self._yield_instrument_controls(self.index) + yield Horizontal( + Button( + "Remove Waypoint", + id=f"wp{self.index}_remove", + variant="error", + ) ) - ) except Exception as e: raise UnexpectedError(unexpected_msg_compose(e)) from None + def get_title(self) -> str: + if isinstance(self.waypoint, Port): + return "Port of Departure" if self.index == 0 else "Port of Arrival" + else: + return f"Waypoint {self.index}" + + def get_copy_button_text(self) -> str: + if isinstance(self.waypoint, Port): + return "Copy Time from Previous" + else: + return "Copy Time & Instruments from Previous" + def get_drifter_count(self) -> int: return sum( 1 for inst in self.waypoint.instrument if inst == InstrumentType.DRIFTER @@ -968,17 +1004,20 @@ def copy_from_previous(self) -> None: else: curr.value = prev.value - for instrument in [ - inst for inst in InstrumentType if not inst.is_underway - ]: - prev_switch = schedule_editor.query_one( - f"#wp{self.index - 1}_{instrument.value}" - ) - curr_switch = self.query_one( - f"#wp{self.index}_{instrument.value}" - ) - if prev_switch and curr_switch: - curr_switch.value = prev_switch.value + if not isinstance( + self.waypoint, Port + ): # only copy instruments for non-port waypoints + for instrument in [ + inst for inst in InstrumentType if not inst.is_underway + ]: + prev_switch = schedule_editor.query_one( + f"#wp{self.index - 1}_{instrument.value}" + ) + curr_switch = self.query_one( + f"#wp{self.index}_{instrument.value}" + ) + if prev_switch and curr_switch: + curr_switch.value = prev_switch.value # hard update self.waypoint.time to match new values as shown in UI year = int(self.query_one(f"#wp{self.index}_year").value) @@ -1083,38 +1122,53 @@ def sync_ui_waypoints(self): """Update the waypoints models with current UI values from the live UI inputs.""" expedition_editor = self.query_one(ExpeditionEditor) errors = [] + for i, wp in enumerate(expedition_editor.expedition.schedule.waypoints): try: - wp.location = Location( - latitude=float(expedition_editor.query_one(f"#wp{i}_lat").value), - longitude=float(expedition_editor.query_one(f"#wp{i}_lon").value), + wp.time = parse_waypoint_datetime( + self.query_one(f"#wp{i}_year", Select).value, + self.query_one(f"#wp{i}_month", Select).value, + self.query_one(f"#wp{i}_day", Select).value, + self.query_one(f"#wp{i}_hour", Select).value, + self.query_one(f"#wp{i}_minute", Select).value, ) - wp.time = datetime.datetime( - int(expedition_editor.query_one(f"#wp{i}_year").value), - int(expedition_editor.query_one(f"#wp{i}_month").value), - int(expedition_editor.query_one(f"#wp{i}_day").value), - int(expedition_editor.query_one(f"#wp{i}_hour").value), - int(expedition_editor.query_one(f"#wp{i}_minute").value), - 0, - ) - wp.instrument = [] - for instrument in [ - inst for inst in InstrumentType if not inst.is_underway - ]: - switch_on = expedition_editor.query_one( - f"#wp{i}_{instrument.value}", Switch - ).value - if instrument.value == "DRIFTER" and switch_on: - count_str = expedition_editor.query_one( - f"#wp{i}_drifter_count", Input + + lat_val = expedition_editor.query_one(f"#wp{i}_lat").value + lon_val = expedition_editor.query_one(f"#wp{i}_lon").value + + if isinstance(wp, Port) and (lat_val == "" or lon_val == ""): + wp.location = Location( + latitude=float(lat_val) if lat_val != "" else None, + longitude=float(lon_val) if lon_val != "" else None, + ) + else: + wp.location = Location( + latitude=float(lat_val), + longitude=float(lon_val), + ) + + if not isinstance(wp, Port): + wp.instrument = [] + + for instrument in [ + inst for inst in InstrumentType if not inst.is_underway + ]: + switch_on = expedition_editor.query_one( + f"#wp{i}_{instrument.value}", Switch ).value - count = int(count_str) - assert count > 0 - wp.instrument.extend([InstrumentType.DRIFTER] * count) - elif switch_on: - wp.instrument.append(instrument) + if instrument.value == "DRIFTER" and switch_on: + count_str = expedition_editor.query_one( + f"#wp{i}_drifter_count", Input + ).value + count = int(count_str) + assert count > 0 + wp.instrument.extend([InstrumentType.DRIFTER] * count) + elif switch_on: + wp.instrument.append(instrument) + except Exception as e: errors.append(f"Waypoint {i + 1}: {e}") + if errors: log_exception_to_file( Exception("\n".join(errors)), @@ -1137,22 +1191,12 @@ def save_pressed(self) -> None: try: ship_speed_value = self.get_ship_speed(expedition_editor) + self.sync_ui_waypoints() - self.sync_ui_waypoints() # call to ensure waypoint inputs are synced - - # verify schedule - _wp_lats, _wp_lons = ( - _get_waypoint_latlons( # TODO: Remove these since they aren't used? - expedition_editor.expedition.schedule.waypoints - ) - ) instruments_config = expedition_editor.expedition.instruments_config + schedule = expedition_editor.expedition.schedule - expedition_editor.expedition.schedule.verify( - ship_speed_value, - instruments_config, - ignore_land_test=True, - ) + schedule.verify(ship_speed_value, instruments_config, ignore_land_test=True) expedition_saved = expedition_editor.save_changes() @@ -1163,13 +1207,23 @@ def save_pressed(self) -> None: timeout=20, ) + # check for incomplete ports and warn the user, but allow save to continue + if ( + not schedule.departure_port.is_in_use + or not schedule.arrival_port.is_in_use + ): + self.notify( + INCOMPLETE_PORT_MSG, + severity="warning", + timeout=20, + ) + except Exception as e: self.notify( - escape( - f"*** Error saving changes ***:\n\n{e}\n" - ), # escape avoids issues with special characters being interpreted as markup + f"*** Error saving changes ***:\n\n{e}\n", severity="error", timeout=20, + markup=False, ) return False diff --git a/src/virtualship/cli/_run.py b/src/virtualship/cli/_run.py index 61e629ce1..18fa922dd 100644 --- a/src/virtualship/cli/_run.py +++ b/src/virtualship/cli/_run.py @@ -10,25 +10,23 @@ from virtualship.expedition.simulate_schedule import ( MeasurementsToSimulate, - ScheduleProblem, simulate_schedule, ) from virtualship.make_realistic.problems.simulator import ProblemSimulator -from virtualship.models import Checkpoint, Schedule +from virtualship.models import Checkpoint from virtualship.models.expedition import Expedition from virtualship.utils import ( CACHE, CHECKPOINT, - EXPEDITION, EXPEDITION_IDENTIFIER, EXPEDITION_LATEST, + INCOMPLETE_PORT_MSG, PROBLEMS_ENCOUNTERED, PROJECTION, REPORT, RESULTS, SELECTED_PROBLEMS, _get_expedition, - _save_checkpoint, expedition_cost, get_instrument_class, ) @@ -78,6 +76,11 @@ def _run( expedition_dir = Path(expedition_dir) expedition = _get_expedition(expedition_dir) + schedule = expedition.schedule + + # warn if the departure and/or arrival port is incomplete + if not schedule.departure_port.is_in_use or not schedule.arrival_port.is_in_use: + print(f"\n{INCOMPLETE_PORT_MSG}") # unique id to determine if an expedition has 'changed' since last run (to avoid re-selecting problems when user makes tweaks to schedule to deal with problems encountered) cache_dir = expedition_dir.joinpath(CACHE) @@ -93,15 +96,14 @@ def _run( # load last checkpoint checkpoint = _load_checkpoint(expedition_dir) - if checkpoint is None: - checkpoint = Checkpoint(past_schedule=Schedule(waypoints=[])) - # verify that schedule and checkpoint match, and that problems have been resolved - checkpoint.verify(expedition, problems_dir) + # verify that schedule and checkpoint match, and that problems have been resolved (if checkpoint exists) + if checkpoint is not None: + checkpoint.verify(expedition, problems_dir) print("\n---- WAYPOINT VERIFICATION ----") - expedition.schedule.verify( + schedule.verify( expedition.ship_config.ship_speed_knots, expedition.instruments_config, from_data=Path(from_data) if from_data else None, @@ -113,20 +115,6 @@ def _run( expedition=expedition, ) - # handle cases where user defined schedule is incompatible (i.e. not enough time between waypoints, not problems) - if isinstance(schedule_results, ScheduleProblem): - print( - f"Please update your schedule (`virtualship plan` or directly in {EXPEDITION}) and continue the expedition by executing the `virtualship run` command again.\nCheckpoint has been saved to {expedition_dir.joinpath(CHECKPOINT)}." - ) - _save_checkpoint( - Checkpoint( - past_schedule=expedition.schedule, - failed_waypoint_i=schedule_results.failed_waypoint_i, - ), - expedition_dir, - ) - return - # delete and create results directory results_dir = expedition_dir.joinpath(RESULTS) _warn_overwrite_results_dir(results_dir) @@ -223,7 +211,7 @@ def _run( ) if problems: - ProblemSimulator.post_expedition_report( + problem_simulator.post_expedition_report( problems, expedition_dir.joinpath(RESULTS, REPORT) ) print("\n----- RECORD OF PROBLEMS ENCOUNTERED ------") @@ -308,10 +296,10 @@ def _load_checkpoint(expedition_dir: Path) -> Checkpoint | None: def _write_expedition_cost(expedition, schedule_results, expedition_dir): """Calculate the expedition cost, write it to a file, and print summary.""" - assert expedition.schedule.waypoints[0].time is not None, ( - "First waypoint has no time. This should not be possible as it should have been verified before." - ) - time_past = schedule_results.time - expedition.schedule.waypoints[0].time + wps_in_use = expedition.schedule._get_wps_in_use() + + assert wps_in_use[0].time is not None, "First waypoint has no time." + time_past = schedule_results.time - wps_in_use[0].time cost = expedition_cost(schedule_results, time_past) with open(expedition_dir.joinpath(RESULTS, "cost.txt"), "w") as file: file.writelines(f"cost: {cost} US$") diff --git a/src/virtualship/cli/commands.py b/src/virtualship/cli/commands.py index 41f4d519e..3442fb1bf 100644 --- a/src/virtualship/cli/commands.py +++ b/src/virtualship/cli/commands.py @@ -2,14 +2,12 @@ import click +from virtualship.cli._initialise import _initialise, _validate_start_date from virtualship.cli._plan import _plan from virtualship.cli._run import _run from virtualship.utils import ( COPERNICUSMARINE_BGC_VARIABLES, COPERNICUSMARINE_PHYS_VARIABLES, - EXPEDITION, - get_example_expedition, - mfp_to_yaml, ) @@ -26,41 +24,21 @@ 'Marine Facilities Planning tool (specifically the "Export Coordinates > DD" option). ' "User edits are required after initialisation.", ) -def init(path, from_mfp): +@click.option( + "--start-date", + type=click.DateTime(formats=["%Y-%m-%d %H:%M:%S", "%Y-%m-%d"]), + default=None, + callback=_validate_start_date, + help="The departure/start date of the expedition (required when using --from-mfp). " + "Expected format: 'YYYY-MM-DD HH:MM:SS' (with quotes, e.g., '2023-10-20 01:00:00'). If only the date is provided, the time will default to 00:00:00.", +) +def init(path, from_mfp, start_date): """ Initialize a directory for a new expedition, with an expedition.yaml file. - If --mfp-file is provided, it will generate the expedition.yaml from the MPF file instead. + If --mfp-file is provided (and --start-date is also provided), it will generate the expedition.yaml from the MPF file instead. """ - path = Path(path) - path.mkdir(exist_ok=True) - - expedition = path / EXPEDITION - - if expedition.exists(): - raise FileExistsError( - f"File '{expedition}' already exist. Please remove it or choose another directory." - ) - - if from_mfp: - mfp_file = Path(from_mfp) - # Generate expedition.yaml from the MPF file - click.echo(f"Generating schedule from {mfp_file}...") - mfp_to_yaml(mfp_file, expedition) - click.echo( - "\n⚠️ The generated schedule does not contain TIME values or INSTRUMENT selections. ⚠️" - "\n\nNow please either use the `\033[4mvirtualship plan\033[0m` app to complete the schedule configuration, " - "\nOR edit 'expedition.yaml' and manually add the necessary time values and instrument selections under the 'schedule' heading." - "\n\nIf editing 'expedition.yaml' manually:" - "\n\n🕒 Expected time format: 'YYYY-MM-DD HH:MM:SS' (e.g., '2023-10-20 01:00:00')." - "\n\n🌡️ Expected instrument(s) format: one line per instrument e.g." - f"\n\n{' ' * 15}waypoints:\n{' ' * 15}- instrument:\n{' ' * 19}- CTD\n{' ' * 19}- ARGO_FLOAT\n" - ) - else: - # Create a default example expedition YAML - expedition.write_text(get_example_expedition()) - - click.echo(f"Created '{expedition.name}' at {path}.") + _initialise(Path(path), from_mfp, start_date) @click.command() diff --git a/src/virtualship/expedition/simulate_schedule.py b/src/virtualship/expedition/simulate_schedule.py index 6af9d80ce..cbf8a4300 100644 --- a/src/virtualship/expedition/simulate_schedule.py +++ b/src/virtualship/expedition/simulate_schedule.py @@ -16,6 +16,7 @@ from virtualship.models import ( Expedition, Location, + Port, Spacetime, Waypoint, ) @@ -30,14 +31,6 @@ class ScheduleOk: measurements_to_simulate: MeasurementsToSimulate -@dataclass -class ScheduleProblem: - """Result of schedule that could not be fully completed.""" - - time: datetime - failed_waypoint_i: int - - @dataclass class MeasurementsToSimulate: """ @@ -68,15 +61,13 @@ def get_attr_for_instrumenttype(cls, instrument_type): xbts: list[XBT] = field(default_factory=list, init=False) -def simulate_schedule( - projection: pyproj.Geod, expedition: Expedition -) -> ScheduleOk | ScheduleProblem: +def simulate_schedule(projection: pyproj.Geod, expedition: Expedition) -> ScheduleOk: """ Simulate a schedule. :param projection: The projection to use for sailing. :param expedition: Expedition object containing the schedule to simulate. - :returns: Either the results of a successfully simulated schedule, or information on where the schedule became infeasible. + :returns: The results of the simulated schedule. """ return _ScheduleSimulator(projection, expedition).simulate() @@ -101,39 +92,31 @@ def __init__(self, projection: pyproj.Geod, expedition: Expedition) -> None: self._projection = projection self._expedition = expedition - assert self._expedition.schedule.waypoints[0].time is not None, ( - "First waypoint must have a time. This should have been verified before calling this function." + assert self._expedition.schedule._verified, ( + "Schedule must be verified before simulation." ) - self._time = expedition.schedule.waypoints[0].time - self._location = expedition.schedule.waypoints[0].location + + self._wps_in_use = self._expedition.schedule._get_wps_in_use() # remove any placeholder departure/arrival ports which are ignored in simulation + self._time = self._wps_in_use[0].time + self._location = self._wps_in_use[0].location self._measurements_to_simulate = MeasurementsToSimulate() self._next_adcp_time = self._time self._next_ship_underwater_st_time = self._time - def simulate(self) -> ScheduleOk | ScheduleProblem: + def simulate(self) -> ScheduleOk: # TODO: instrument config mapping (as introduced in #269) should be helpful for refactoring here (i.e. #236)... - for wp_i, waypoint in enumerate(self._expedition.schedule.waypoints): + for waypoint in self._wps_in_use: # sail towards waypoint self._progress_time_traveling_towards(waypoint.location) - # check if waypoint was reached in time - # TODO: already tested in schedule.verify(), re-check here for robustness but could be removed if deemed redundant - if waypoint.time is not None and self._time > waypoint.time: - print( - f"\nWaypoint {wp_i + 1} could not be reached in time. Current time: {self._time}. Waypoint time: {waypoint.time}." - "\n\nHave 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" - ) - return ScheduleProblem(self._time, wp_i) - else: - self._time = ( - waypoint.time - ) # wait at the waypoint until ship is scheduled to be there + # wait at the waypoint until ship is scheduled to be there + self._time = waypoint.time # note measurements made at waypoint - time_passed = self._make_measurements(waypoint) + time_passed = self._get_instrument_timescosts(waypoint) # wait while measurements are being done self._progress_time_stationary(time_passed) @@ -247,9 +230,13 @@ def _get_underway_stationary_times( for i in range(1, int(npts) + 1) ] - def _make_measurements(self, waypoint: Waypoint) -> timedelta: - # if there are no instruments, there is no time cost - if waypoint.instrument is None: + 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 diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index f13a83388..5126bcd69 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -3,12 +3,12 @@ import abc import collections import inspect +import itertools import tempfile from dataclasses import dataclass -from datetime import timedelta -from itertools import pairwise +from datetime import datetime, timedelta from pathlib import Path -from typing import TYPE_CHECKING, ClassVar, Literal +from typing import TYPE_CHECKING, Any, ClassVar, Literal import copernicusmarine import numpy as np @@ -26,7 +26,7 @@ _find_files_in_timerange, _find_nc_file_with_variable, _get_bathy_data, - _get_instrument_relevant_waypoints, + _get_instr_relevant_wps, _get_waypoint_latlons, _select_product_id, _SpinnerAutoStop, @@ -35,7 +35,44 @@ if TYPE_CHECKING: from virtualship.instruments.sensors import SensorType - from virtualship.models import Expedition + from virtualship.models import Expedition, Waypoint + + +@dataclass(frozen=True) +class SpatialBounds: + """Spatio-temporal bounding box for instrument's fieldset.""" + + min_lat: float + max_lat: float + min_lon: float + max_lon: float + min_time: datetime + max_time: datetime + + @classmethod + def from_waypoints(cls, waypoints: list[Waypoint]) -> SpatialBounds: + """Create a SpatialBounds instance from a list of waypoints.""" + lats, lons = _get_waypoint_latlons(waypoints) + times = [wp.time for wp in waypoints if wp.time is not None] + return cls( + min_lat=min(lats), + max_lat=max(lats), + min_lon=min(lons), + max_lon=max(lons), + min_time=times[0], + max_time=times[-1] + timedelta(days=1), # avoid edge issues + ) + + def with_buffer( + self, latlon_buffer: float = 0.0 + ) -> tuple[float, float, float, float]: + """Return (min_lon, max_lon, min_lat, max_lat) including optional spatial buffer.""" + return ( + self.min_lon - latlon_buffer, + self.max_lon + latlon_buffer, + self.min_lat - latlon_buffer, + self.max_lat + latlon_buffer, + ) @dataclass @@ -56,7 +93,7 @@ class Instrument(abc.ABC): sensor_kernels: ClassVar[dict[SensorType, collections.abc.Callable]] def __init_subclass__(cls, **kwargs: object) -> None: - """Ensure non-abstract subclasses (i.e. final/concrete instrument classes) define sensor_kernels as a class attribute.""" + """Ensure concrete instrument subclasses define required class attributes.""" super().__init_subclass__(**kwargs) if inspect.isabstract(cls): return @@ -69,7 +106,7 @@ def __init_subclass__(cls, **kwargs: object) -> None: def __init__( self, expedition: Expedition, - variables: dict, + variables: dict[str, Any], add_bathymetry: bool, verbose_progress: bool, from_data: Path | None, @@ -78,31 +115,27 @@ def __init__( """Initialise instrument.""" self.expedition = expedition self.from_data = from_data - self.variables = collections.OrderedDict(variables) self.add_bathymetry = add_bathymetry self.verbose_progress = verbose_progress - self.fetch_spec = fetch_spec or FetchSpec() + self.fetch_spec = fetch_spec if fetch_spec is not None else FetchSpec() self._tmp_dirs: list[tempfile.TemporaryDirectory] = [] - # only waypoints relevant to this instrument; avoid needlessly ballooning fieldset to full expedition schedule - relevant_waypoints = _get_instrument_relevant_waypoints( - expedition.schedule.waypoints, self.instrument_type - ) + # filter to waypoints relevant to this instrument + wps_in_use = self.expedition.schedule._get_wps_in_use() + relevant_waypoints = _get_instr_relevant_wps(wps_in_use, self.instrument_type) + if not relevant_waypoints: + raise ValueError( + f"No relevant waypoints found for instrument '{self.instrument_type}'." + ) - wp_lats, wp_lons = _get_waypoint_latlons(relevant_waypoints) + # verify time ordering wp_times = [wp.time for wp in relevant_waypoints if wp.time is not None] - assert all(earlier <= later for earlier, later in pairwise(wp_times)), ( - "Waypoint times are not in ascending order" - ) - self.wp_times = wp_times + if not all(a <= b for a, b in itertools.pairwise(wp_times)): + raise ValueError("Relevant waypoint times are not in ascending order.") - self.min_time, self.max_time = ( - wp_times[0], - wp_times[-1] + timedelta(days=1), - ) # avoid edge issues - self.min_lat, self.max_lat = min(wp_lats), max(wp_lats) - self.min_lon, self.max_lon = min(wp_lons), max(wp_lons) + # spatio-temporal bounding box of all relevant waypoints + self.bounds = SpatialBounds.from_waypoints(relevant_waypoints) def close(self): """Explicitly cleanup all tmp dirs.""" @@ -195,7 +228,6 @@ def _generate_fieldset(self) -> parcels.FieldSet: """ combined_fieldset = None keys = list(self.variables.keys()) - time_buffer = self.fetch_spec.time_buffer for key in keys: @@ -207,8 +239,8 @@ def _generate_fieldset(self) -> parcels.FieldSet: files = _find_files_in_timerange( data_dir, - self.min_time, - self.max_time + timedelta(days=time_buffer), + self.bounds.min_time, + self.bounds.max_time + timedelta(days=time_buffer), ) _, field_var_name = _find_nc_file_with_variable( @@ -252,13 +284,17 @@ def _get_copernicus_ds( """Get Copernicus Marine dataset for direct ingestion.""" product_id = _select_product_id( physical=physical, - schedule_start=self.min_time, - schedule_end=self.max_time, + schedule_start=self.bounds.min_time, + schedule_end=self.bounds.max_time, variable=var if not physical else None, ) - # spatial bounds with buffer, if spatial constraints apply - min_lon_wbuf, max_lon_wbuf, min_lat_wbuf, max_lat_wbuf = self.spatial_bounds + buf = self.fetch_spec.latlon_buffer if self.fetch_spec.spatial else 0.0 + min_lon, max_lon, min_lat, max_lat = ( + self.bounds.with_buffer(buf) + if self.fetch_spec.spatial + else (None, None, None, None) + ) min_depth = ( abs(self.fetch_spec.depth_min) @@ -273,13 +309,13 @@ def _get_copernicus_ds( return copernicusmarine.open_dataset( dataset_id=product_id, - minimum_longitude=min_lon_wbuf, - maximum_longitude=max_lon_wbuf, - minimum_latitude=min_lat_wbuf, - maximum_latitude=max_lat_wbuf, + minimum_longitude=min_lon, + maximum_longitude=max_lon, + minimum_latitude=min_lat, + maximum_latitude=max_lat, variables=[var], - start_datetime=self.min_time, - end_datetime=self.max_time + timedelta(days=time_buffer), + start_datetime=self.bounds.min_time, + end_datetime=self.bounds.max_time + timedelta(days=time_buffer), minimum_depth=min_depth, maximum_depth=max_depth, coordinates_selection_method="outside", @@ -303,9 +339,8 @@ def _get_local_ds(self, files: list[Path]) -> xr.Dataset: f"Missing or invalid 'positive' attribute for 'depth' coordinate in {files[0].parent}. Expected 'positive: up' or 'positive: down'. Original error: {e}" ) from e - # sel only relevant latlon and depth subsets, to speed up simulations (avoid bringing in potentially global data) - # spatial bounds with buffer, if spatial constraints apply - min_lon_wbuf, max_lon_wbuf, min_lat_wbuf, max_lat_wbuf = self.spatial_bounds + buf = self.fetch_spec.latlon_buffer if self.fetch_spec.spatial else 0.0 + min_lon, max_lon, min_lat, max_lat = self.bounds.with_buffer(buf) depth_min = self.fetch_spec.depth_min depth_max = self.fetch_spec.depth_max @@ -315,20 +350,16 @@ def _get_local_ds(self, files: list[Path]) -> xr.Dataset: depth_sel = { "depth": [depth_min], "method": "nearest", - } # preserve depth dim with square brackets + } else: - # max, min slice because depth is negative and positive: up depth_sel = {"depth": slice(depth_max, depth_min)} ds = ds.sel( - longitude=slice(min_lon_wbuf, max_lon_wbuf), - latitude=slice(min_lat_wbuf, max_lat_wbuf), + longitude=slice(min_lon, max_lon), + latitude=slice(min_lat, max_lat), ) - # separate sel (from lat, lon above) for depth to allow `nearest` selection if not using slices - # will leave as is if both_none, as intended ds = ds.sel(**depth_sel) - return ds def _via_tmp_ds(self, ds: xr.Dataset) -> xr.Dataset: @@ -337,12 +368,10 @@ def _via_tmp_ds(self, ds: xr.Dataset) -> xr.Dataset: self._tmp_dirs.append(tmp_dir) tmp_store = Path(tmp_dir.name) / f"tmp_{id(ds)}.zarr" - # strip pre-existing per-variable encoding, which may interfere with zarr defaults ds_to_write = ds.copy() for variable in ds_to_write.variables.values(): variable.encoding = {} - # TODO: potential trade off between speed and memory usage here... could remove to reduce memory footprint, but may slow down writing (?) ds_to_write = ds_to_write.chunk( {dim: size for dim, size in ds_to_write.sizes.items()} ) @@ -386,22 +415,6 @@ def instrument_type(self) -> InstrumentType: """Return the InstrumentType for this instrument instance.""" return next(k for k, v in INSTRUMENT_CLASS_MAP.items() if type(self) is v) - @property - def spatial_bounds( - self, - ) -> tuple[float | None, float | None, float | None, float | None]: - """Return (min_lon, max_lon, min_lat, max_lat) bounds including buffer if spatial constraints apply.""" - if not self.fetch_spec.spatial: - return None, None, None, None - - buf = self.fetch_spec.latlon_buffer - return ( - self.min_lon - buf, - self.max_lon + buf, - self.min_lat - buf, - self.max_lat + buf, - ) - @dataclass(frozen=True) class UnderwayCoordinates: diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index 87d6a92e8..2206a1c63 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -1,10 +1,9 @@ from __future__ import annotations -import json -import os import random import sys import time +from datetime import timedelta from pathlib import Path from typing import TYPE_CHECKING @@ -23,6 +22,7 @@ InstrumentProblem, ) from virtualship.models.checkpoint import Checkpoint +from virtualship.models.expedition import Port from virtualship.utils import ( CACHE, EXPEDITION, @@ -31,8 +31,11 @@ PROJECTION, _calc_sail_time, _calc_wp_stationkeeping_time, + _get_public_wp, _make_hash, + _read_json, _save_checkpoint, + _write_json, ) if TYPE_CHECKING: @@ -41,386 +44,315 @@ LOG_MESSAGING = { "pre_departure": "Hang on! There could be a pre-departure problem in-port...", "during_expedition": "Oh no, a problem has occurred during the expedition, at waypoint {waypoint}...!", - "schedule_problems": "This problem will cause a delay of {delay_duration} hours {problem_wp}. The next waypoint therefore cannot be reached in time. Please account for this in your schedule (`virtualship plan` or directly in {expedition_yaml}), then continue the expedition by executing the `virtualship run` command again.\n", + "schedule_problems": ( + "This problem will cause a delay of {delay_duration} hours {problem_wp}. " + "The next waypoint therefore cannot be reached in time. Please account for this " + "in your schedule (`virtualship plan` or directly in {expedition_yaml}), then continue " + "the expedition by executing the `virtualship run` command again.\n" + ), "problem_avoided": "Phew! You had enough contingency time scheduled to avoid delays from this problem.\n", } - -# default problem weights for problems simulator (i.e. add +1 problem for every n days/waypoints/instruments in expedition) +# default problem weights for problems simulator (e.g., +1 problem every N days/waypoints/instruments) PROBLEM_WEIGHTS = { "every_ndays": 7, "every_nwaypoints": 6, "every_ninstruments": 3, } +ProblemType = GeneralProblem | InstrumentProblem +SelectedProblemsDict = dict[str, list[ProblemType | None]] + class ProblemSimulator: - """Handle problem simulation during expedition.""" + """Handle problem simulation during an expedition.""" def __init__(self, expedition: Expedition, expedition_dir: str | Path): """Initialise ProblemSimulator with a schedule and probability level.""" self.expedition = expedition self.expedition_dir = Path(expedition_dir) + self.waypoints = expedition.schedule.waypoints + + # version with inactive Ports (if any) filtered out + self.wps_in_use = self.expedition.schedule._get_wps_in_use() def select_problems( self, instruments_in_expedition: set[InstrumentType], difficulty_level: str, - ) -> dict[str, list[GeneralProblem | InstrumentProblem] | None] | None: + ) -> SelectedProblemsDict | None: """ Select problems (general and instrument-specific). When difficulty_level = 'hard', number of problems is determined by expedition length, instrument count etc. If only one waypoint, return just a pre-departure problem. - Map each selected problem to a random waypoint (or None if pre-departure). Finally, cache the suite of problems to a directory (expedition-specific) for reference. + Map each selected problem to a random waypoint (or 0th [i.e. departure port] if pre-departure). """ if difficulty_level == "easy": return None - valid_instrument_problems = [ - problem - for problem in INSTRUMENT_PROBLEMS - if problem.instrument_type in instruments_in_expedition - ] + # isolate only the non-port waypoints + num_non_port_wps = sum(1 for wp in self.wps_in_use if not isinstance(wp, Port)) - pre_departure_problems = [ + # if only one waypoint, return just a pre-departure problem + if num_non_port_wps < 2: + pre_departure = [p for p in GENERAL_PROBLEMS if p.pre_departure] + return { + "problem_class": [random.choice(pre_departure)], + # pre-departure problem is always associated with the departure port (index 0) + "waypoint_i": [0], + } + + valid_instruments = [ p - for p in GENERAL_PROBLEMS - if isinstance(p, GeneralProblem) and p.pre_departure + for p in INSTRUMENT_PROBLEMS + if p.instrument_type in instruments_in_expedition ] - num_waypoints = len(self.expedition.schedule.waypoints) - num_instruments = len(instruments_in_expedition) - expedition_duration_days = ( - self.expedition.schedule.waypoints[-1].time - - self.expedition.schedule.waypoints[0].time - ).days + # use all waypoints (incl. Ports) here + num_problems = self._calculate_problem_count( + difficulty_level=difficulty_level, + expedition_days=(self.wps_in_use[-1].time - self.wps_in_use[0].time).days, + num_waypoints=len(self.wps_in_use), + num_instruments=len(instruments_in_expedition), + max_available=len(GENERAL_PROBLEMS) + len(valid_instruments), + ) - # if only one waypoint, return just a pre-departure problem - if num_waypoints < 2: - return { - "problem_class": [random.choice(pre_departure_problems)], - "waypoint_i": [None], - } + selected = self._sample_problems( + num_problems, valid_instruments, len(instruments_in_expedition) + ) + selected = self._limit_pre_departure(selected, valid_instruments) + + return self._assign_problems_to_waypoints(selected) + + def _calculate_problem_count( + self, + difficulty_level: str, + expedition_days: int, + num_waypoints: int, + num_instruments: int, + max_available: int, + ) -> int: + """Determine problem count based on difficulty setting.""" + assert difficulty_level != "easy", ( + "Easy difficulty level should not call for a problem count." + ) if difficulty_level == "medium": - num_problems = random.randint(1, 2) + return random.randint(1, 2) elif difficulty_level == "hard": - base = 1 - extra = ( # i.e. +1 problem for every n days/waypoints/instruments (tunable above) - (expedition_duration_days // PROBLEM_WEIGHTS["every_ndays"]) + extra = ( + (expedition_days // PROBLEM_WEIGHTS["every_ndays"]) + (num_waypoints // PROBLEM_WEIGHTS["every_nwaypoints"]) + (num_instruments // PROBLEM_WEIGHTS["every_ninstruments"]) ) - num_problems = base + extra - num_problems = min( - num_problems, len(GENERAL_PROBLEMS) + len(valid_instrument_problems) - ) + return min(1 + extra, max_available) - assert num_problems > 0, ( - f"Difficulty mode is: {difficulty_level}, but no problems were selected." - ) + def _sample_problems( + self, + num_problems: int, + valid_instruments: list[InstrumentProblem], + num_instruments: int, + ) -> list[ProblemType]: + """Sample a balanced ratio of general and instrument problems.""" + general_pool = list(GENERAL_PROBLEMS) + instrument_pool = list(valid_instruments) + random.shuffle(general_pool) + random.shuffle(instrument_pool) + + bias = min(0.7, num_instruments / (num_instruments + 2)) + n_inst = round(num_problems * bias) + n_gen = min(len(general_pool), num_problems - n_inst) + # recalc in case n_gen was capped to len(GENERAL_PROBLEMS) + n_inst = num_problems - n_gen + + return general_pool[:n_gen] + instrument_pool[:n_inst] + + def _limit_pre_departure( + self, + selected: list[ProblemType], + valid_instruments: list[InstrumentProblem], + ) -> list[ProblemType]: + """Ensure maximum of one pre-departure problem is selected.""" + pre_deps = [ + p for p in selected if isinstance(p, GeneralProblem) and p.pre_departure + ] + if len(pre_deps) <= 1: + return selected + + keep = random.choice(pre_deps) + replacements_needed = len(pre_deps) - 1 + filtered = [ + p for p in selected if p is keep or not getattr(p, "pre_departure", False) + ] - selected_problems = [] - problems_sorted = None - random.shuffle(GENERAL_PROBLEMS) - random.shuffle(valid_instrument_problems) + avail_gen = [ + p for p in GENERAL_PROBLEMS if not p.pre_departure and p not in filtered + ] + avail_inst = [p for p in valid_instruments if p not in filtered] + replacements = avail_gen + avail_inst + random.shuffle(replacements) + + return filtered + replacements[:replacements_needed] + + def _assign_problems_to_waypoints( + self, selected: list[ProblemType] + ) -> SelectedProblemsDict | None: + """Assign sampled problems to valid, non-port waypoint indices.""" + waypoints = self.waypoints + avail_indices = [ + i for i, wp in enumerate(waypoints) if not isinstance(wp, Port) + ] + random.shuffle(avail_indices) - # bias towards more instrument problems when there are more instruments - instrument_bias = min(0.7, num_instruments / (num_instruments + 2)) - n_instrument = round(num_problems * instrument_bias) - n_general = min(len(GENERAL_PROBLEMS), num_problems - n_instrument) - n_instrument = ( - num_problems - n_general - ) # recalc in case n_general was capped to len(GENERAL_PROBLEMS) + assigned_problems: list[ProblemType] = [] + assigned_indices: list[int | None] = [] - selected_problems.extend(GENERAL_PROBLEMS[:n_general]) - selected_problems.extend(valid_instrument_problems[:n_instrument]) + has_active_departure_port = self.expedition.schedule.departure_port.is_in_use - # allow only one pre-departure problem to occur; replace any extras with non-pre-departure problems - selected_pre_departure = [ - p - for p in selected_problems - if isinstance(p, GeneralProblem) and p.pre_departure - ] - if len(selected_pre_departure) > 1: - to_keep = random.choice(selected_pre_departure) - num_to_replace = len(selected_pre_departure) - 1 - # remove all but one pre_departure problem - selected_problems = [ - problem - for problem in selected_problems - if not ( - isinstance(problem, GeneralProblem) - and problem.pre_departure - and problem is not to_keep - ) - ] - # available non-pre_departure problems not already selected - available_general = [ - p - for p in GENERAL_PROBLEMS - if not p.pre_departure and p not in selected_problems - ] - available_instrument = [ - p for p in valid_instrument_problems if p not in selected_problems - ] - available_replacements = available_general + available_instrument - random.shuffle(available_replacements) - selected_problems.extend(available_replacements[:num_to_replace]) - - # map each problem to a [random] waypoint (or None if pre-departure) - # limited to one per waypoint, else complicates scheduling and contingency checking - waypoint_idxs = [] - unassigned_problems = [] - available_idxs = list( - range(len(self.expedition.schedule.waypoints) - 1) - ) # exclude last waypoint (problem there would have no impact on scheduling) - - # TODO: if incorporate departure and arrival port/waypoints in future, bear in mind index selection here may need to change - for problem in selected_problems: + for problem in selected: if getattr(problem, "pre_departure", False): - waypoint_idxs.append(None) + assigned_problems.append(problem) + + # index is 0 if there is an active departure port, otherwise None (no waypoint associated with pre-departure problem) + assigned_indices.append(0 if has_active_departure_port else None) + continue + + if not avail_indices: + break + + # find matching waypoint or substitute with general problem + target_idx = None + for idx in avail_indices: + wp_instruments = waypoints[idx].instrument or [] + if ( + isinstance(problem, InstrumentProblem) + and problem.instrument_type not in wp_instruments + ): + continue + target_idx = idx + break + + if target_idx is not None: + avail_indices.remove(target_idx) + assigned_problems.append(problem) + assigned_indices.append(target_idx) else: - if available_idxs: - wp_select = random.choice(available_idxs) - - # fmt: off - # check waypoint actually deploys the instrument associated with the problem...if not, replace it with a general (non-instrument related) problem - # rather than a different waypoint, because it's possible no applicable waypoint is still available - wp_instruments = self.expedition.schedule.waypoints[wp_select].instrument - if isinstance(problem, InstrumentProblem) and problem.instrument_type not in wp_instruments: - available_general = [p for p in GENERAL_PROBLEMS if not p.pre_departure and p not in selected_problems] - - if not available_general: - unassigned_problems.append(problem) - continue - - replacement = random.choice(available_general) - problem_idx = selected_problems.index(problem) - selected_problems[problem_idx] = replacement - # fmt: on - - waypoint_idxs.append(wp_select) - available_idxs.remove(wp_select) # each waypoint only used once - - else: - unassigned_problems.append( - problem - ) # if run out of available waypoints, remove problem from selection - - # remove any problems that couldn't be assigned a waypoint (i.e. if more problems than available waypoints) - if unassigned_problems: - selected_problems = [ - p for p in selected_problems if p not in unassigned_problems - ] - - # pair problems with their waypoint indices and sort by waypoint index (pre-departure first) + # fall back to a general problem if instrument match fails + avail_general = [ + p + for p in GENERAL_PROBLEMS + if not p.pre_departure and p not in assigned_problems + ] + if avail_general and avail_indices: + substitute = random.choice(avail_general) + assigned_problems.append(substitute) + assigned_indices.append(avail_indices.pop()) + + if not assigned_problems: + return None + + # sort chronologically (waypoint 0/None first, then remaining waypoint index order) paired = sorted( - zip(selected_problems, waypoint_idxs, strict=True), - key=lambda x: (x[1] is not None, x[1] if x[1] is not None else -1), + zip(assigned_problems, assigned_indices, strict=True), + key=lambda x: (x[1] is not None, x[1]), ) - problems_sorted = { + return { "problem_class": [p for p, _ in paired], "waypoint_i": [w for _, w in paired], } - return problems_sorted if selected_problems else None - def execute( self, - problems: dict[str, list[GeneralProblem | InstrumentProblem] | None], + problems: SelectedProblemsDict, instrument_type_validation: InstrumentType | None, log_dir: Path, log_delay: float = 4.0, - ): - """ - Execute the selected problems, returning messaging and delay times. - - N.B. a problem_waypoint_i is different to a failed_waypoint_i defined in the Checkpoint class; failed_waypoint_i is the waypoint index after the problem_waypoint_i where the problem occurred, as this is when scheduling issues would be encountered. - """ - # TODO: when difficulty_level = 'hard' and have general problems which occur at later waypoints: could artificially delay their propagation until later in the simulation? Otherwise they are front-loaded at the start of the simulation... Instrument problems are fine because they only propagate when instrument is simulated... - - for problem, problem_waypoint_i in zip( + ) -> None: + """Execute simulation problems and apply delay/schedule impacts.""" + for problem, wp_i in zip( problems["problem_class"], problems["waypoint_i"], strict=True ): - # skip if instrument problem but `p.instrument_type` does not match `instrument_type_validation` (i.e. the current instrument being simulated in the expedition, e.g. from _run.py) if ( isinstance(problem, InstrumentProblem) and problem.instrument_type is not instrument_type_validation ): continue - problem_hash = _make_hash(problem.message + str(problem_waypoint_i), 8) - hash_fpath = log_dir.joinpath(f"problem_{problem_hash}.json") + problem_hash = _make_hash(problem.message + str(wp_i), 8) + hash_fpath = log_dir / f"problem_{problem_hash}.json" if hash_fpath.exists(): - continue # problem * waypoint combination has already occurred; don't repeat - - if isinstance(problem, GeneralProblem) and problem.pre_departure: - alert_msg = LOG_MESSAGING["pre_departure"] - - else: - alert_msg = LOG_MESSAGING["during_expedition"].format( - waypoint=int(problem_waypoint_i) + 1 - ) - - # log problem occurrence, save to checkpoint, and pause simulation - self._log_problem( - problem, - problem_waypoint_i, - alert_msg, - problem_hash, - hash_fpath, - log_delay, - ) + continue - # cache original expedition for reference and/or restoring later if needed (checkpoint.yaml [written in _log_problem] can be overwritten if multiple problems occur so is not a persistent record of original schedule) + self._log_problem(problem, wp_i, problem_hash, hash_fpath, log_delay) self._cache_original_expedition(self.expedition) - @staticmethod - def cache_selected_problems( - problems: dict[str, list[GeneralProblem | InstrumentProblem] | None], - selected_problems_fpath: str, - ) -> None: - """Cache suite of problems to json, for reference.""" - # make dir to contain problem jsons (unique to expedition) - os.makedirs(Path(selected_problems_fpath).parent, exist_ok=True) - - # cache dict of selected_problems to json - with open( - selected_problems_fpath, - "w", - encoding="utf-8", - ) as f: - json.dump( - { - "problem_class": [p.short_name for p in problems["problem_class"]], - "waypoint_i": problems["waypoint_i"], - "timestamp": time.strftime("%Y-%m-%d %H:%M:%S", time.localtime()), - }, - f, - indent=4, - ) - - @staticmethod - def post_expedition_report( - problems: dict[str, list[GeneralProblem | InstrumentProblem] | None], - report_fpath: str | Path, - ) -> None: - """Produce human-readable post-expedition report (.txt), including problems that occured (their full messages), the waypoint and what delay they caused.""" - for problem, problem_waypoint_i in zip( - problems["problem_class"], problems["waypoint_i"], strict=True - ): - affected_wp = ( - "in-port" if problem_waypoint_i is None else f"{problem_waypoint_i + 1}" - ) - delay_hours = problem.delay_duration.total_seconds() / 3600.0 - with open(report_fpath, "a", encoding="utf-8") as f: - f.write("---\n") - f.write(f"Waypoint: {affected_wp}\n") - f.write(f"Problem: {problem.message}\n") - f.write(f"Delay caused: {delay_hours} hours\n\n") - - @staticmethod - def load_selected_problems( - selected_problems_fpath: str, - ) -> dict[str, list[GeneralProblem | InstrumentProblem] | None]: - """Load previously selected problem classes from json.""" - with open( - selected_problems_fpath, - encoding="utf-8", - ) as f: - problems_json = json.load(f) - - # extract selected problem classes from their names (using the lookups preserves order they were saved in) - selected_problems = {"problem_class": [], "waypoint_i": []} - general_problems_lookup = {cls.short_name: cls for cls in GENERAL_PROBLEMS} - instrument_problems_lookup = { - cls.short_name: cls for cls in INSTRUMENT_PROBLEMS - } - - for cls_name, wp_idx in zip( - problems_json["problem_class"], problems_json["waypoint_i"], strict=True - ): - if cls_name in general_problems_lookup: - selected_problems["problem_class"].append( - general_problems_lookup[cls_name] - ) - elif cls_name in instrument_problems_lookup: - selected_problems["problem_class"].append( - instrument_problems_lookup[cls_name] - ) - else: - raise ValueError( - f"Problem class '{cls_name}' not found in known problem registries." - ) - selected_problems["waypoint_i"].append(wp_idx) - - return selected_problems - def _log_problem( self, - problem: GeneralProblem | InstrumentProblem, - problem_waypoint_i: int | None, - alert_msg: str, + problem: ProblemType, + problem_wp_i: int | None, problem_hash: str, hash_fpath: Path, log_delay: float, - ): - """Log problem occurrence with spinner and delay, save to checkpoint, write hash.""" - time.sleep(3.0) # brief pause before spinner + ) -> None: + """ + Handle execution sequence, logging, checkpoint saving, and user presentation. + + Note, problem_wp_i is the index of the waypoint in the expedition schedule, but the user-facing message should be based on the index of the waypoint in the list of non-port waypoints. + Use problem_wp_i for internal logic, but user-facing messages (below) should use public_wp (non indexed version). + problem_wp_i will often == public_wp (given 0-indexing), but this makes the logic explicit and clear. + """ + waypoints = self.waypoints + + if problem_wp_i is None: + # pre-departure problem but no active Port in the schedule, so pretend the problem is at departure port for user messaging + public_wp = None + else: + public_wp = _get_public_wp(problem_wp_i, waypoints) + + alert_msg = ( + LOG_MESSAGING["pre_departure"] + if isinstance(problem, GeneralProblem) and problem.pre_departure + else LOG_MESSAGING["during_expedition"].format(waypoint=public_wp) + ) + + time.sleep(3.0) with yaspin(text=alert_msg) as spinner: time.sleep(log_delay) spinner.ok("💥 ") - self._hash_to_json( - problem, - problem_hash, - problem_waypoint_i, - hash_fpath, - ) - - has_contingency = self._has_contingency(problem, problem_waypoint_i) + self._hash_to_json(problem, problem_hash, problem_wp_i, hash_fpath) + has_contingency = self._has_contingency(problem, problem_wp_i) + delay_hrs = problem.delay_duration.total_seconds() / 3600.0 if has_contingency: impact_str = LOG_MESSAGING["problem_avoided"] result_str = "The expedition will carry on shortly as planned." - - # update problem json to resolved = True - with open(hash_fpath, encoding="utf-8") as f: - problem_json = json.load(f) - problem_json["resolved"] = True - with open(hash_fpath, "w", encoding="utf-8") as f_out: - json.dump(problem_json, f_out, indent=4) - + # update problem JSON state to resolved + data = _read_json(hash_fpath) + data["resolved"] = True + _write_json(hash_fpath, data) else: - affected = ( - "in-port" - if problem_waypoint_i is None - else f"at waypoint {problem_waypoint_i + 1}" + affected = "in-port" if public_wp is None else f"at waypoint {public_wp}" + impact_str = ( + f"Not enough contingency time scheduled to mitigate delay of {delay_hrs} " + f"hours occurring {affected} (future waypoint(s) would be reached too late).\n" ) - - impact_str = f"Not enough contingency time scheduled to mitigate delay of {problem.delay_duration.total_seconds() / 3600.0} hours occuring {affected} (future waypoint(s) would be reached too late).\n" result_str = LOG_MESSAGING["schedule_problems"].format( - delay_duration=problem.delay_duration.total_seconds() / 3600.0, + delay_duration=delay_hrs, problem_wp=affected, expedition_yaml=EXPEDITION, ) - # save checkpoint + # update and save checkpoints checkpoint = Checkpoint( past_schedule=self.expedition.schedule, - failed_waypoint_i=problem_waypoint_i + 1 - if problem_waypoint_i is not None - else 0, - ) # failed waypoint index then becomes the one after the one where the problem occurred; as this is when scheduling issues would be run into; for pre-departure problems this is the first waypoint + problem_wp_i=problem_wp_i, + ) _save_checkpoint(checkpoint, self.expedition_dir) + self.expedition.to_yaml(self.expedition_dir / CACHE / EXPEDITION_LATEST) - # save latest version of expedition (overwrites previous) - self.expedition.to_yaml(self.expedition_dir.joinpath(CACHE, EXPEDITION_LATEST)) - - # display tabular output in self._tabular_outputter( problem_str=problem.message, impact_str=impact_str, @@ -428,124 +360,161 @@ def _log_problem( has_contingency=has_contingency, ) - if has_contingency: - return # continue expedition as normal - else: - sys.exit(0) # pause simulation + if not has_contingency: + sys.exit(0) - def _has_contingency( - self, - problem: InstrumentProblem | GeneralProblem, - problem_waypoint_i: int | None, - ) -> bool: - """Determine if enough contingency time has been scheduled to avoid delay affecting the waypoint immediately after the problem.""" - if problem_waypoint_i is None: - return False # pre-departure problems always cause delay to first waypoint + def _has_contingency(self, problem: ProblemType, problem_wp_i: int | None) -> bool: + """Check whether scheduled contingency covers expected delay duration.""" + # special case where pretending that a pre-departure problem is at the departure port but there is no active Port in the schedule (problem_wp_i = None) + # always returns False, as there is no way to determine whether there is enough contingency time in this case + is_pre_departure_no_active_port = ( + getattr(problem, "pre_departure", False) and problem_wp_i is None + ) + if is_pre_departure_no_active_port: + return False - else: - curr_wp = self.expedition.schedule.waypoints[problem_waypoint_i] - next_wp = self.expedition.schedule.waypoints[problem_waypoint_i + 1] + curr_wp, next_wp = ( + self.waypoints[problem_wp_i], + self.waypoints[problem_wp_i + 1], + ) - wp_stationkeeping_time = _calc_wp_stationkeeping_time( - curr_wp.instrument, self.expedition - ) + stationkeeping = ( + _calc_wp_stationkeeping_time(curr_wp.instrument, self.expedition) + if not isinstance(curr_wp, Port) + else timedelta(0) + ) + sail_time = _calc_sail_time( + curr_wp.location, + next_wp.location, + ship_speed_knots=self.expedition.ship_config.ship_speed_knots, + projection=PROJECTION, + )[0] - scheduled_time_diff = next_wp.time - curr_wp.time + scheduled_time = next_wp.time - curr_wp.time + required_time = sail_time + stationkeeping + problem.delay_duration - sail_time = _calc_sail_time( - curr_wp.location, - next_wp.location, - ship_speed_knots=self.expedition.ship_config.ship_speed_knots, - projection=PROJECTION, - )[0] + return scheduled_time > required_time - return ( - scheduled_time_diff - > sail_time + wp_stationkeeping_time + problem.delay_duration - ) + def _cache_original_expedition(self, expedition: Expedition) -> None: + """Cache original schedule configuration to file for recovery.""" + path = self.expedition_dir / CACHE / EXPEDITION_ORIGINAL + if not path.exists(): + expedition.to_yaml(path) + print(f"\nOriginal expedition.yaml cached to {path}.\n") - def _make_checkpoint(self, failed_waypoint_i: int | None = None) -> Checkpoint: - """Make checkpoint, also handling pre-departure.""" - return Checkpoint( - past_schedule=self.expedition.schedule, failed_waypoint_i=failed_waypoint_i - ) + @staticmethod + def cache_selected_problems( + problems: SelectedProblemsDict, selected_problems_fpath: str | Path + ) -> None: + """Cache suite of selected problems to JSON.""" + fpath = Path(selected_problems_fpath) + fpath.parent.mkdir(parents=True, exist_ok=True) + + payload = { + "problem_class": [p.short_name for p in problems["problem_class"]], + "waypoint_i": problems["waypoint_i"], + "timestamp": time.strftime("%Y-%m-%d %H:%M:%S", time.localtime()), + } + _write_json(fpath, payload) + + @staticmethod + def load_selected_problems( + selected_problems_fpath: str | Path, + ) -> SelectedProblemsDict: + """Load selected problems suite from a cached JSON file.""" + data = _read_json(Path(selected_problems_fpath)) + + general_lookup = {cls.short_name: cls for cls in GENERAL_PROBLEMS} + instrument_lookup = {cls.short_name: cls for cls in INSTRUMENT_PROBLEMS} + + selected_classes, waypoint_indices = [], [] + for cls_name, wp_idx in zip( + data["problem_class"], data["waypoint_i"], strict=True + ): + if cls_name in general_lookup: + selected_classes.append(general_lookup[cls_name]) + elif cls_name in instrument_lookup: + selected_classes.append(instrument_lookup[cls_name]) + else: + raise ValueError( + f"Problem class '{cls_name}' not found in known registries." + ) + waypoint_indices.append(wp_idx) - def _cache_original_expedition(self, expedition: Expedition): - """Cache original schedule to file for user's reference.""" - path = self.expedition_dir.joinpath(CACHE, EXPEDITION_ORIGINAL) - if path.exists(): - return # don't overwrite if already cached - expedition.to_yaml(path) - print(f"\nOriginal expedition.yaml cached to {path}.\n") + return {"problem_class": selected_classes, "waypoint_i": waypoint_indices} + + def post_expedition_report( + self, problems: SelectedProblemsDict, report_fpath: str | Path + ) -> None: + """Append human-readable report summary of all occurring problems.""" + with open(report_fpath, "a", encoding="utf-8") as f: + for problem, wp_i in zip( + problems["problem_class"], problems["waypoint_i"], strict=True + ): + # None means pre-departure with no active departure port + public_wp = ( + None if wp_i is None else _get_public_wp(wp_i, self.waypoints) + ) + affected = "in-port" if public_wp is None else f"{public_wp}" + delay_hrs = problem.delay_duration.total_seconds() / 3600.0 + f.write( + f"---\nWaypoint: {affected}\n" + f"Problem: {problem.message}\n" + f"Delay caused: {delay_hrs} hours\n\n" + ) @staticmethod def _hash_to_json( - problem: InstrumentProblem | GeneralProblem, + problem: ProblemType, problem_hash: str, - problem_waypoint_i: int | None, + problem_wp_i: int | None, hash_path: Path, - ) -> dict: - """Convert problem details + hash to json.""" + ) -> None: + """Serialize runtime problem detail to JSON.""" hash_data = { "problem_hash": problem_hash, "message": problem.message, - "problem_waypoint_i": problem_waypoint_i, + "problem_wp_i": problem_wp_i, "delay_duration_hours": problem.delay_duration.total_seconds() / 3600.0, "timestamp": time.strftime("%Y-%m-%d %H:%M:%S", time.localtime()), "resolved": False, } - with open(hash_path, "w", encoding="utf-8") as f: - json.dump(hash_data, f, indent=4) + _write_json(hash_path, hash_data) @staticmethod - def _tabular_outputter(problem_str, impact_str, result_str, has_contingency: bool): + def _tabular_outputter( + problem_str: str, impact_str: str, result_str: str, has_contingency: bool + ) -> None: """Display the problem, impact, and result in a live-updating table. Sleep times are included to increase readability and engagement for user.""" console = Console() console.print() # line break before table - col_kwargs = dict(ratio=1, no_wrap=False, max_width=None, justify="left") + col_kwargs = dict(ratio=1, no_wrap=False, justify="left") - def make_table(problem, impact, result, col_kwargs, colour_results=False): + def make_table(problem, impact, result, colour_results=False) -> Table: table = Table(box=box.SIMPLE, expand=True) table.add_column("Problem Encountered", **col_kwargs) table.add_column("Impact on schedule", **col_kwargs) - if colour_results: - style = "green1" if has_contingency else "red1" - table.add_column("Result", style=style, **col_kwargs) - else: - table.add_column("Result", **col_kwargs) - + style = ( + ("green1" if has_contingency else "red1") if colour_results else None + ) + table.add_column("Result", style=style, **col_kwargs) table.add_row(problem, impact, result) return table - empty_spinner = Spinner("dots", text="") + empty = Spinner("dots", text="") impact_spinner = Spinner("dots", text="Assessing impact on schedule...") + stages = [ + (empty, empty, empty, False, 3.0), + (problem_str, empty, empty, False, 3.0), + (problem_str, impact_spinner, empty, False, 7.0), + (problem_str, impact_str, empty, False, 4.0), + (problem_str, impact_str, result_str, True, 3.0), + ] + with Live(console=console, refresh_per_second=10) as live: - # stage 0: empty table - table = make_table(empty_spinner, empty_spinner, empty_spinner, col_kwargs) - live.update(table) - time.sleep(3.0) - - # stage 1: show problem - table = make_table(problem_str, empty_spinner, empty_spinner, col_kwargs) - live.update(table) - time.sleep(3.0) - - # stage 2: spinner in "Impact on schedule" column - table = make_table(problem_str, impact_spinner, empty_spinner, col_kwargs) - live.update(table) - time.sleep(7.0) - - # stage 3: table with problem and impact-investigation complete - table = make_table(problem_str, impact_str, empty_spinner, col_kwargs) - live.update(table) - time.sleep(4.0) - - # stage 4: complete table with problem, impact, and result (give final outcome colour based on fail/success) - table = make_table( - problem_str, impact_str, result_str, col_kwargs, colour_results=True - ) - live.update(table) - time.sleep(3.0) + for prob, imp, res, colour, sleep_time in stages: + live.update(make_table(prob, imp, res, colour_results=colour)) + time.sleep(sleep_time) diff --git a/src/virtualship/models/__init__.py b/src/virtualship/models/__init__.py index dd4b2bf14..b95544c89 100644 --- a/src/virtualship/models/__init__.py +++ b/src/virtualship/models/__init__.py @@ -8,6 +8,7 @@ DrifterConfig, Expedition, InstrumentsConfig, + Port, Schedule, SensorConfig, ShipConfig, @@ -23,6 +24,7 @@ __all__ = [ # noqa: RUF022 "Location", + "Port", "Schedule", "SensorConfig", "ShipConfig", diff --git a/src/virtualship/models/checkpoint.py b/src/virtualship/models/checkpoint.py index ce620af1f..3c70b13ff 100644 --- a/src/virtualship/models/checkpoint.py +++ b/src/virtualship/models/checkpoint.py @@ -2,7 +2,6 @@ from __future__ import annotations -import json from datetime import timedelta from pathlib import Path @@ -11,12 +10,15 @@ from virtualship.errors import CheckpointError from virtualship.instruments.types import InstrumentType -from virtualship.models.expedition import Expedition, Schedule +from virtualship.models.expedition import Expedition, Port, Schedule from virtualship.utils import ( EXPEDITION, PROJECTION, _calc_sail_time, _calc_wp_stationkeeping_time, + _get_public_wp, + _read_json, + _write_json, ) @@ -37,7 +39,7 @@ class Checkpoint(pydantic.BaseModel): """ past_schedule: Schedule - failed_waypoint_i: int | None = None + problem_wp_i: int | None = None def to_yaml(self, file_path: str | Path) -> None: """ @@ -68,118 +70,98 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: """ new_schedule = expedition.schedule - # 1) check that past waypoints have not been changed, unless is a pre-departure problem - if self.failed_waypoint_i is None: - pass - elif ( - not new_schedule.waypoints[: int(self.failed_waypoint_i)] - == self.past_schedule.waypoints[: int(self.failed_waypoint_i)] + # problem_wp_i is None for a pre-departure problem where the departure port is an inactive placeholder + # so there is no real problem waypoint to anchor timing calculations to + has_problem_location = self.problem_wp_i is not None + problem_wp_i = self.problem_wp_i if has_problem_location else 0 + + # failed waypoint is the waypoint immediately *after* the problem waypoint (i.e. the one that will not be reached in time) + failed_wp_i = problem_wp_i + 1 + + # public waypoint number of problem and failed waypoints, for use in error messages + public_problem_wp = ( + _get_public_wp(problem_wp_i, self.past_schedule.waypoints) + if has_problem_location + else None + ) + public_failed_wp = _get_public_wp(failed_wp_i, self.past_schedule.waypoints) + + # 1) check that past waypoints have not been changed (up to but not including failed_wp) + if ( + not new_schedule.waypoints[:failed_wp_i] + == self.past_schedule.waypoints[:failed_wp_i] ): raise CheckpointError( - f"Past waypoints in schedule have been changed! Restore past schedule and only change future waypoints (waypoint {int(self.failed_waypoint_i) + 1} onwards)." + f"Past waypoints in schedule have been changed! Restore past schedule and only change future waypoints (waypoint {public_failed_wp} onwards)." ) # 2) check that problems have been resolved in the new schedule + failed_waypoint = new_schedule.waypoints[failed_wp_i] + + if has_problem_location: + problem_waypoint = new_schedule.waypoints[problem_wp_i] + + stationkeeping_time = ( + _calc_wp_stationkeeping_time(problem_waypoint.instrument, expedition) + if not isinstance(problem_waypoint, Port) + else timedelta(0) + ) + + sail_time = _calc_sail_time( + problem_waypoint.location, + failed_waypoint.location, + ship_speed_knots=expedition.ship_config.ship_speed_knots, + projection=PROJECTION, + )[0] + + available_time = failed_waypoint.time - problem_waypoint.time + base_time = problem_waypoint.time + fixed_delay_offset = sail_time + stationkeeping_time + else: + # no departure location/time to sail from (departure port is an inactive placeholder) + base_time = self.past_schedule.waypoints[failed_wp_i].time + available_time = failed_waypoint.time - base_time + fixed_delay_offset = timedelta(0) + hash_fpaths = [ str(path.resolve()) for path in problems_dir.glob("problem_*.json") ] - if len(hash_fpaths) > 0: - for file in hash_fpaths: - with open(file, encoding="utf-8") as f: - problem = json.load(f) - if problem["resolved"]: - continue - elif not problem["resolved"]: - # check if delay has been accounted for in the new schedule (at waypoint immediately after problem waypoint; or first waypoint if pre-departure problem) - delay_duration = timedelta( - hours=float(problem["delay_duration_hours"]) - ) - - problem_waypoint = ( - new_schedule.waypoints[0] - if problem["problem_waypoint_i"] is None - else new_schedule.waypoints[problem["problem_waypoint_i"]] + for file in hash_fpaths: + problem = _read_json(file) + + # continue if problem is already resolved, else perform checks to see if delay is accounted for + if problem["resolved"]: + continue + + delay_duration = timedelta(hours=float(problem["delay_duration_hours"])) + min_time_required = fixed_delay_offset + delay_duration + expected_arrival = base_time + min_time_required + + if available_time >= min_time_required: + print("\n\n🎉 Previous problem has been resolved in the schedule.\n") + + # save back to json file changing the resolved status to True + problem["resolved"] = True + _write_json(file, problem) + + # only handle the first unresolved problem found; others will be handled in subsequent runs but are not yet known to the user + break + + else: + problem_wp_str = ( + "in-port" + if public_problem_wp is None + else f"at waypoint {public_problem_wp}" + ) + + raise CheckpointError( + f"The problem encountered in previous simulation has not been resolved in the schedule! Please adjust the schedule to account for delays caused by the problem (by using `virtualship plan` or directly editing the {EXPEDITION} file).\n\n" + f"The problem was associated with a delay duration of {problem['delay_duration_hours']} hours {problem_wp_str} (meaning waypoint {public_failed_wp} could not be reached in time). " + f"Currently, the ship would reach waypoint {public_failed_wp} at {expected_arrival}, but the scheduled time is {failed_waypoint.time}." + + ( + f"\n\nHint: don't forget to factor in the time required to deploy the instruments {problem_wp_str} when rescheduling waypoint {public_failed_wp}." + if public_problem_wp is not None + else "" ) - - # pre-departure problem: check that whole delay duration has been added to first waypoint time (by testing against past schedule) - if problem["problem_waypoint_i"] is None: - time_diff = ( - problem_waypoint.time - self.past_schedule.waypoints[0].time - ) - resolved = time_diff >= delay_duration - - # problem at a later waypoint: check new scheduled time exceeds sail time + delay duration + instrument deployment time (rather whole delay duration add-on, as there may be _some_ contingency time already scheduled) - else: - failed_waypoint = new_schedule.waypoints[self.failed_waypoint_i] - - scheduled_time = failed_waypoint.time - problem_waypoint.time - - stationkeeping_time = _calc_wp_stationkeeping_time( - problem_waypoint.instrument, - expedition, - ) # total time required to deploy instruments at problem waypoint - - sail_time = _calc_sail_time( - problem_waypoint.location, - failed_waypoint.location, - ship_speed_knots=expedition.ship_config.ship_speed_knots, - projection=PROJECTION, - )[0] - - min_time_required = ( - sail_time + delay_duration + stationkeeping_time - ) - - resolved = scheduled_time >= min_time_required - - if resolved: - print( - "\n\n🎉 Previous problem has been resolved in the schedule.\n" - ) - - # save back to json file changing the resolved status to True - problem["resolved"] = True - with open(file, "w", encoding="utf-8") as f_out: - json.dump(problem, f_out, indent=4) - - # only handle the first unresolved problem found; others will be handled in subsequent runs but are not yet known to the user - break - - else: - problem_wp_str = ( - "in-port" - if problem["problem_waypoint_i"] is None - else f"at waypoint {problem['problem_waypoint_i'] + 1}" - ) - affected_wp_str = ( - "1" - if problem["problem_waypoint_i"] is None - else f"{problem['problem_waypoint_i'] + 2}" - ) - time_elapsed = ( - (sail_time + delay_duration + stationkeeping_time) - if problem["problem_waypoint_i"] is not None - else delay_duration - ) - failed_waypoint_time = ( - failed_waypoint.time - if problem["problem_waypoint_i"] is not None - else new_schedule.waypoints[0].time - ) - current_time = ( - problem_waypoint.time + time_elapsed - if problem["problem_waypoint_i"] is not None - else self.past_schedule.waypoints[0].time + time_elapsed - ) - - raise CheckpointError( - f"The problem encountered in previous simulation has not been resolved in the schedule! Please adjust the schedule to account for delays caused by the problem (by using `virtualship plan` or directly editing the {EXPEDITION} file).\n\n" - f"The problem was associated with a delay duration of {problem['delay_duration_hours']} hours {problem_wp_str} (meaning waypoint {affected_wp_str} could not be reached in time). " - f"Currently, the ship would reach waypoint {affected_wp_str} at {current_time}, but the scheduled time is {failed_waypoint_time}." - + ( - f"\n\nHint: don't forget to factor in the time required to deploy the instruments {problem_wp_str} when rescheduling waypoint {affected_wp_str}." - if problem["problem_waypoint_i"] is not None - else "" - ) - ) + ) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 79d45abb9..c65e137ef 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -3,7 +3,7 @@ import itertools from datetime import datetime, timedelta from pathlib import Path -from typing import ClassVar +from typing import ClassVar, cast import numpy as np import pydantic @@ -17,6 +17,7 @@ _calc_sail_time, _calc_wp_stationkeeping_time, _get_bathy_data, + _get_public_wp, _validate_numeric_to_timedelta, get_supported_sensors, register_instrument_config, @@ -37,9 +38,11 @@ class Expedition(pydantic.BaseModel): model_config = pydantic.ConfigDict(extra="forbid") def to_yaml(self, file_path: str) -> None: - """Write exepedition object to yaml file.""" + """Write expedition object to yaml file, with port/waypoint number comments.""" + annotated = self._annotate() + with open(file_path, "w") as file: - yaml.dump(self.model_dump(by_alias=True), file) + file.writelines(annotated) @classmethod def from_yaml(cls, file_path: str) -> Expedition: @@ -51,8 +54,11 @@ def from_yaml(cls, file_path: str) -> Expedition: def get_instruments(self) -> set[InstrumentType]: """Return a set of unique InstrumentType enums used in the expedition.""" instruments_in_expedition = [] + # from waypoints for waypoint in self.schedule.waypoints: + if isinstance(waypoint, Port): + continue if waypoint.instrument: for instrument in waypoint.instrument: if instrument: @@ -70,6 +76,30 @@ def get_instruments(self) -> set[InstrumentType]: "Underway instrument config attribute(s) are missing from YAML. Must be Config object or None." ) from e + def _annotate(self): + """Add port/waypoint comments/annotations to the expedition.yaml file.""" + raw = yaml.dump(self.model_dump(by_alias=True), default_flow_style=False) + + lines = raw.splitlines(keepends=True) + annotated = [] + waypoint_number = 0 + for line in lines: + stripped = line.lstrip() + indent = " " * (len(line) - len(stripped)) + + # waypoints start with "- instrument:" and Ports start with "- location:" (no instrument field). + if stripped.startswith("- instrument:"): + waypoint_number += 1 + annotated.append(f"{indent}# Waypoint {waypoint_number}\n") + + if stripped.startswith("- location:"): + arrival_departure = "Departure" if waypoint_number == 0 else "Arrival" + annotated.append(f"{indent}# Port of {arrival_departure}\n") + + annotated.append(line) + + return annotated + class ShipConfig(pydantic.BaseModel): """Configuration of the ship.""" @@ -84,9 +114,34 @@ class ShipConfig(pydantic.BaseModel): class Schedule(pydantic.BaseModel): """Schedule of the virtual ship.""" - waypoints: list[Waypoint] - + waypoints: list[Port | Waypoint] model_config = pydantic.ConfigDict(extra="forbid") + _verified: bool = False # internal flag to indicate if the schedule has been verified, so that a schedule can be simulated safely elsewhere in codebase + + @pydantic.field_validator("waypoints", mode="after") + @classmethod + def _validate_waypoints(cls, value: list[Port | Waypoint]) -> list[Port | Waypoint]: + """Ensure first and last waypoints are Port objects and schedule contains non-port waypoints.""" + if not isinstance(value[0], Port) or not isinstance(value[-1], Port): + raise ScheduleError( + "First and last waypoints must be Ports (of arrival/departure). " + "One or the other is currently missing." + ) + + if not any(isinstance(wp, Waypoint) for wp in value): + raise ScheduleError("At least one non-port waypoint must be provided.") + + return value + + @property + def departure_port(self) -> Port: + """Departure port (always the first waypoint).""" + return cast(Port, self.waypoints[0]) + + @property + def arrival_port(self) -> Port: + """Arrival port (always the last waypoint).""" + return cast(Port, self.waypoints[-1]) def verify( self, @@ -96,24 +151,17 @@ def verify( *, from_data: Path | None = None, ) -> None: - """ - Verify the feasibility and correctness of the schedule's waypoints. - - This method checks various conditions to ensure the schedule is valid: - 1. At least one waypoint is provided. - 2. The first waypoint has a specified time. - 3. Waypoint times are in ascending order. - 4. All waypoints are in water (not on land). - 5. The ship can arrive on time at each waypoint given its speed. - """ + """Verify the feasibility and correctness of the schedule's waypoints.""" print("\nVerifying route... ") - if len(self.waypoints) == 0: - raise ScheduleError("At least one waypoint must be provided.") + # waypoints excluding any inactive placeholder departure/arrival ports + wps_in_use = self._get_wps_in_use() - # check first waypoint has a time - if self.waypoints[0].time is None: - raise ScheduleError("First waypoint must have a specified time.") + # 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: + raise ScheduleError(f"{wp_str} must have a specified time.") # check waypoint times are in ascending order timed_waypoints = [wp for wp in self.waypoints if wp.time is not None] @@ -122,11 +170,12 @@ def verify( ] if not all(checks): invalid_i = [i for i, c in enumerate(checks) if c] + public_wps = [_get_public_wp(i, self.waypoints) for i in invalid_i] raise ScheduleError( - f"Waypoint(s) {', '.join(f'#{i + 1}' for i in invalid_i)}: each waypoint should be timed after all previous waypoints", + f"Waypoint(s) {', '.join(f'#{i}' for i in public_wps)}: each waypoint should be timed after all previous waypoints", ) - # check if all waypoints are in water using bathymetry data + # check if all non-port waypoints are in water using bathymetry data land_waypoints = [] if not ignore_land_test: try: @@ -137,6 +186,9 @@ def verify( ) 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 @@ -145,24 +197,24 @@ def verify( wp.location.lon, ) if value == 0.0 or (isinstance(value, float) and np.isnan(value)): - land_waypoints.append((wp_i, wp)) + land_waypoints.append((public_wp, wp)) except Exception as e: raise ScheduleError( - f"Waypoint #{wp_i + 1} at location {wp.location} could not be evaluated against bathymetry data. \n\n Original error: {e}" + 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(wp_i + 1) + ' ' + str(wp) for (wp_i, wp) in land_waypoints]}\n\nINFO: They are likely on land (bathymetry data cannot be interpolated to their location(s)).\n" + 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 = self.waypoints[0].time - for wp_i, (wp, wp_next) in enumerate( - zip(self.waypoints, self.waypoints[1:], strict=False) - ): + time = wps_in_use[0].time + + for wp_i, (wp, wp_next) in enumerate(itertools.pairwise(wps_in_use)): stationkeeping_time = _calc_wp_stationkeeping_time( - wp.instrument, instruments_config + wp.instrument if isinstance(wp, Waypoint) else None, + instruments_config, ) time_to_reach = _calc_sail_time( @@ -177,16 +229,57 @@ def verify( 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 waypoint {wp_i + 2}. " + 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 + end_slice = ( + len(self.waypoints) + if self.arrival_port.is_in_use + else len(self.waypoints) - 1 + ) + return self.waypoints[start_slice:end_slice] + + +class Port(pydantic.BaseModel): + """A port stop: a location the ship visits with no instrument deployments made.""" + + location: Location | None = None + time: datetime | None = None + + model_config = pydantic.ConfigDict(extra="forbid") + + @property + def is_in_use(self) -> bool: + """Return True if the port has both a valid time and location (lat/lon).""" + return ( + self.time is not None + and self.location is not None + and self.location.lat is not None + and self.location.lon is not None + ) + class Waypoint(pydantic.BaseModel): """A Waypoint to sail to with an optional time and an optional instrument.""" diff --git a/src/virtualship/models/location.py b/src/virtualship/models/location.py index 793e5312c..1c40bb8b4 100644 --- a/src/virtualship/models/location.py +++ b/src/virtualship/models/location.py @@ -7,26 +7,29 @@ class Location: """A location on a sphere.""" - latitude: float - longitude: float + latitude: float | None = None + longitude: float | None = None def __post_init__(self) -> None: """ - Verify this location has valid latitude and longitude. + Verify this location has valid latitude and longitude if provided. :raises ValueError: If latitude and/or longitude are not valid. """ - if self.lat < -90: - raise ValueError("Latitude cannot be smaller than -90.") - if self.lat > 90: - raise ValueError("Latitude cannot be larger than 90.") - if self.lon < -180: - raise ValueError("Longitude cannot be smaller than -180.") - if self.lon > 360: - raise ValueError("Longitude cannot be larger than 360.") + if self.lat is not None: + if self.lat < -90: + raise ValueError("Latitude cannot be smaller than -90.") + if self.lat > 90: + raise ValueError("Latitude cannot be larger than 90.") + + if self.lon is not None: + if self.lon < -180: + raise ValueError("Longitude cannot be smaller than -180.") + if self.lon > 360: + raise ValueError("Longitude cannot be larger than 360.") @property - def lat(self) -> float: + def lat(self) -> float | None: """ Shorthand for latitude variable. @@ -35,7 +38,7 @@ def lat(self) -> float: return self.latitude @property - def lon(self) -> float: + def lon(self) -> float | None: """ Shorthand for longitude variable. diff --git a/src/virtualship/static/expedition.yaml b/src/virtualship/static/expedition.yaml index acb16dcf0..1e201543c 100644 --- a/src/virtualship/static/expedition.yaml +++ b/src/virtualship/static/expedition.yaml @@ -1,36 +1,5 @@ # see https://virtualship.readthedocs.io/en/latest/user-guide/tutorials/working_with_expedition_yaml.html for more details on how to edit this file # -schedule: - waypoints: - - instrument: - - CTD - location: - latitude: 0 - longitude: 0 - time: 1998-01-01 00:00:00 - - instrument: - - DRIFTER - - CTD - location: - latitude: 0.01 - longitude: 0.01 - time: 1998-01-02 01:00:00 - - instrument: - - ARGO_FLOAT - location: - latitude: 0.02 - longitude: 0.02 - time: 1998-01-03 02:00:00 - - instrument: - - XBT - location: - latitude: 0.03 - longitude: 0.03 - time: 1998-01-04 03:00:00 - - location: - latitude: 0.03 - longitude: 0.03 - time: 1998-01-05 03:00:00 instruments_config: adcp_config: num_bins: 40 @@ -82,5 +51,52 @@ instruments_config: sensors: - TEMPERATURE - SALINITY +schedule: + waypoints: + # Port of Departure + - location: + latitude: 0 + longitude: 0 + time: 1998-01-01 00:00:00 + # Waypoint 1 + - instrument: + - CTD + location: + latitude: 0.01 + longitude: 0.01 + time: 1998-01-02 00:00:00 + # Waypoint 2 + - instrument: + - DRIFTER + - CTD + location: + latitude: 0.02 + longitude: 0.02 + time: 1998-01-03 01:00:00 + # Waypoint 3 + - instrument: + - ARGO_FLOAT + location: + latitude: 0.03 + longitude: 0.03 + time: 1998-01-04 02:00:00 + # Waypoint 4 + - instrument: + - XBT + location: + latitude: 0.04 + longitude: 0.04 + time: 1998-01-05 03:00:00 + # Waypoint 5 + - instrument: [] + location: + latitude: 0.05 + longitude: 0.05 + time: 1998-01-06 04:00:00 + # Port of Arrival + - location: + latitude: 0.06 + longitude: 0.06 + time: 1998-01-07 05:00:00 ship_config: ship_speed_knots: 10.0 diff --git a/src/virtualship/utils.py b/src/virtualship/utils.py index a3c7ba6e8..604e46395 100644 --- a/src/virtualship/utils.py +++ b/src/virtualship/utils.py @@ -2,10 +2,9 @@ import glob import hashlib -import os +import json import re import sys -import warnings from datetime import datetime, timedelta from functools import lru_cache from importlib.resources import files @@ -29,7 +28,6 @@ from virtualship.models.checkpoint import Checkpoint from virtualship.models.expedition import SensorConfig -import pandas as pd import yaml from pydantic import BaseModel from yaspin import Spinner @@ -100,6 +98,16 @@ BATHYMETRY_ID = "cmems_mod_glo_phy_my_0.083deg_static" +# ===================================================== +# SECTION: warnings and messages +# ===================================================== + + +INCOMPLETE_PORT_MSG = ( + "WARNING: Departure and/or arrival port is/are incomplete in the schedule (missing time, location, or both). " + "The simulation will continue but incomplete ports will be ignored." +) + # ===================================================== # SECTION: decorators / dynamic registries and mapping @@ -158,16 +166,15 @@ def decorator(cls): # ===================================================== -def load_static_file(name: str) -> str: +def _load_static_file(name: str) -> str: """Load static file from the ``virtualship.static`` module by file name.""" return files("virtualship.static").joinpath(name).read_text(encoding="utf-8") @lru_cache(None) -@lru_cache(None) -def get_example_expedition() -> str: +def _get_example_expedition() -> str: """Get the example unified expedition configuration file.""" - return load_static_file(EXPEDITION) + return _load_static_file(EXPEDITION) def _dump_yaml(model: BaseModel, stream: TextIO) -> str | None: @@ -182,137 +189,6 @@ def _generic_load_yaml(data: str, model: BaseModel) -> BaseModel: return model.model_validate(yaml.safe_load(data)) -def load_coordinates(file_path): - """Loads coordinates from a file based on its extension.""" - if not os.path.isfile(file_path): - raise FileNotFoundError(f"File not found: {file_path}") - - ext = os.path.splitext(file_path)[-1].lower() - - try: - if ext in [".xls", ".xlsx"]: - return pd.read_excel(file_path) - - if ext == ".csv": - return pd.read_csv(file_path) - - raise ValueError(f"Unsupported file extension {ext}.") - - except Exception as e: - raise RuntimeError( - "Could not read coordinates data from the provided file. " - "Ensure it is either a csv or excel file." - ) from e - - -def validate_coordinates(coordinates_data): - # Expected column headers - expected_columns = {"Station Type", "Name", "Latitude", "Longitude"} - - # Check if the headers match the expected ones - actual_columns = set(coordinates_data.columns) - - missing_columns = expected_columns - actual_columns - if missing_columns: - raise ValueError( - f"Error: Found columns {list(actual_columns)}, but expected columns {list(expected_columns)}. " - "Are you sure that you're using the correct export from MFP?" - ) - - extra_columns = actual_columns - expected_columns - if extra_columns: - warnings.warn( - f"Found additional unexpected columns {list(extra_columns)}. " - "Manually added columns have no effect. " - "If the MFP export format changed, please submit an issue: " - "https://github.com/OceanParcels/virtualship/issues.", - stacklevel=2, - ) - - # Drop unexpected columns (optional, only if you want to ensure strict conformity) - coordinates_data = coordinates_data[list(expected_columns)] - - # Continue with the rest of the function after validation... - coordinates_data = coordinates_data.dropna() - - # Convert latitude and longitude to floats, replacing commas with dots - # Handles case when the latitude and longitude have decimals with commas - if coordinates_data["Latitude"].dtype in ["object", "string"]: - coordinates_data["Latitude"] = coordinates_data["Latitude"].apply( - lambda x: float(x.replace(",", ".")) - ) - - if coordinates_data["Longitude"].dtype in ["object", "string"]: - coordinates_data["Longitude"] = coordinates_data["Longitude"].apply( - lambda x: float(x.replace(",", ".")) - ) - - return coordinates_data - - -def mfp_to_yaml(coordinates_file_path: str, yaml_output_path: str): # noqa: D417 - """ - Generates an expedition.yaml file with schedule information based on data from MFP excel file. The ship and instrument configurations entries in the YAML file are sourced from the static version. - - Parameters - ---------- - - excel_file_path (str): Path to the Excel file containing coordinate and instrument data. - - The function: - 1. Reads instrument and location data from the Excel file. - 2. Determines the maximum depth and buffer based on the instruments present. - 3. Ensures longitude and latitude values remain valid after applying buffer adjustments. - 4. returns the yaml information. - - """ - # avoid circular imports - from virtualship.models import ( - Expedition, - InstrumentsConfig, - Location, - Schedule, - Waypoint, - ) - - # Read data from file - coordinates_data = load_coordinates(coordinates_file_path) - - coordinates_data = validate_coordinates(coordinates_data) - - # Generate waypoints - waypoints = [] - for _, row in coordinates_data.iterrows(): - waypoints.append( - Waypoint( - instrument=None, # instruments blank, to be built by user using `virtualship plan` UI or by interacting directly with YAML files - location=Location(latitude=row["Latitude"], longitude=row["Longitude"]), - ) - ) - - # Create Schedule object - schedule = Schedule( - waypoints=waypoints, - ) - - # extract instruments config from static - instruments_config = InstrumentsConfig.model_validate( - yaml.safe_load(get_example_expedition()).get("instruments_config") - ) - - # extract ship config from static - ship_config = yaml.safe_load(get_example_expedition()).get("ship_config") - - # combine to Expedition object - expedition = Expedition( - schedule=schedule, - instruments_config=instruments_config, - ship_config=ship_config, - ) - - # Save to YAML file - expedition.to_yaml(yaml_output_path) - - def _validate_numeric_to_timedelta( value: int | float | timedelta, unit: Literal["minutes", "days"] ) -> timedelta: @@ -592,20 +468,23 @@ def _get_waypoint_latlons(waypoints): return wp_lats, wp_lons -def _get_instrument_relevant_waypoints(waypoints, instrument_type) -> list: +def _get_instr_relevant_wps(waypoints, instrument_type) -> list: """Subset of waypoints that are relevant to this `instrument_type`.""" + from virtualship.models import Port # avoid circular import problems + if instrument_type.is_underway: return list(waypoints) relevant = [] for wp in waypoints: - wp_instruments = ( - wp.instrument - if isinstance(wp.instrument, list) - else ([wp.instrument] if wp.instrument else []) - ) - if instrument_type in wp_instruments: - relevant.append(wp) + if not isinstance(wp, Port): + wp_instruments = ( + wp.instrument + if isinstance(wp.instrument, list) + else ([wp.instrument] if wp.instrument else []) + ) + if instrument_type in wp_instruments: + relevant.append(wp) return relevant or list(waypoints) @@ -615,6 +494,16 @@ def _save_checkpoint(checkpoint: Checkpoint, expedition_dir: Path) -> None: checkpoint.to_yaml(file_path) +def _read_json(path: Path) -> dict: + with open(path, encoding="utf-8") as f: + return json.load(f) + + +def _write_json(path: Path, data: dict) -> None: + with open(path, "w", encoding="utf-8") as f: + json.dump(data, f, indent=4) + + def _calc_sail_time( location1: Location, location2: Location, @@ -638,7 +527,7 @@ def _calc_sail_time( def _calc_wp_stationkeeping_time( - wp_instrument_types: list, + wp_instrument_types: list | None, instruments_config: InstrumentsConfig, instrument_config_map: dict = INSTRUMENT_CONFIG_MAP, ) -> timedelta: @@ -692,6 +581,25 @@ def build_particle_class_from_sensors( return Particle.add_variable(nonsensor_variables + sensor_variables) +def _get_public_wp(raw_wp_i: int, waypoints: list) -> int | None: + """ + Get the public waypoint number for a given raw waypoint index (accounting for Port waypoints). + + Note, the returned number is not an index, rather it corresponds to Waypoint numbers ignoring Ports (which are not waypoints from the user's perspective). + """ + from virtualship.models.expedition import Port # avoid circular import + + port_wps = [i for i, wp in enumerate(waypoints) if isinstance(wp, Port)] + non_port_wps = [i for i in range(len(waypoints)) if i not in port_wps] + + if raw_wp_i in port_wps: + public_wp = None # Port waypoints do not have public waypoint numbers + else: + public_wp = non_port_wps.index(raw_wp_i) + 1 + + return public_wp + + # ===================================================== # SECTION: misc. # ===================================================== diff --git a/tests/cli/test_initialise.py b/tests/cli/test_initialise.py new file mode 100644 index 000000000..0037c7b6d --- /dev/null +++ b/tests/cli/test_initialise.py @@ -0,0 +1,202 @@ +import pandas as pd +import pytest + +from virtualship.cli._initialise import _mfp_to_yaml +from virtualship.models import Expedition, Port, Waypoint +from virtualship.utils import _get_example_expedition + + +def test_get_example_expedition(): + assert len(_get_example_expedition()) > 0 + + +def test_valid_example_expedition(tmp_path): + path = tmp_path / "test.yaml" + with open(path, "w") as file: + file.write(_get_example_expedition()) + + Expedition.from_yaml(path) + + +def valid_mfp_data(): + return pd.DataFrame( + { + "Station": [ + "Departure Port", + "Station1", + "Station2", + "Station3", + "Arrival Port", + ], + "Type": ["Departure Port", "CTD", "CTD", "CTD", "Arrival Port"], + "Latitude": [30.8, 31.2, 32.5, 33.1, 34.0], + "Longitude": [-44.3, -45.1, -46.7, -47.2, -48.0], + "Sea Depth": [100, 200, 300, 400, 500], + "Time at Station": [ + "0d 00h 00m", + "0d 01h 00m", + "0d 01h 00m", + "0d 01h 00m", + "0d 00h 00m", + ], + "Travel Time to Next": [ + "0d 05h 00m", + "0d 06h 00m", + "0d 04h 00m", + "0d 03h 00m", + None, + ], + "Distance to Next (NM)": [50, 60, 40, 30, None], + "Ship Speed (kn)": [10, 10, 10, 10, None], + "EEZ": ["EEZ1", "EEZ1", "EEZ2", "EEZ2", "EEZ2"], + } + ) + + +@pytest.fixture +def valid_excel_mfp_file(tmp_path): + path = tmp_path / "file.xlsx" + valid_mfp_data().to_excel(path, index=False) + return path + + +@pytest.fixture +def valid_excel_mfp_file_with_commas(tmp_path): + path = tmp_path / "file.xlsx" + df = valid_mfp_data() + df["Latitude"] = df["Latitude"].astype(str).str.replace(".", ",") + df["Longitude"] = df["Longitude"].astype(str).str.replace(".", ",") + df.to_excel(path, index=False) + return path + + +@pytest.fixture +def invalid_mfp_file(tmp_path): + """File missing required MFP columns.""" + path = tmp_path / "file.xlsx" + df = pd.DataFrame({"WrongColumn": [1, 2, 3]}) + df.to_excel(path, index=False) + return path + + +@pytest.fixture +def unsupported_extension_mfp_file(tmp_path): + path = tmp_path / "file.unsupported" + valid_mfp_data().to_csv(path, index=False) + return path + + +@pytest.fixture +def nonexistent_mfp_file(tmp_path): + return tmp_path / "non_file.xlsx" + + +@pytest.fixture +def missing_columns_mfp_file(tmp_path): + path = tmp_path / "file.xlsx" + valid_mfp_data().drop(columns=["Longitude"]).to_excel(path, index=False) + return path + + +@pytest.fixture +def missing_ports_mfp_file(tmp_path): + path = tmp_path / "file.xlsx" + # remove rows marked as departure or arrival ports + df = valid_mfp_data() + df = df[~df["Station"].str.contains("Port")] + df.to_excel(path, index=False) + return path + + +@pytest.fixture +def unexpected_header_mfp_file(tmp_path): + path = tmp_path / "file.xlsx" + df = valid_mfp_data() + df["Unexpected Column"] = ["Extra1", "Extra2", "Extra3", "Extra4", "Extra5"] + df.to_excel(path, index=False) + return path + + +@pytest.mark.parametrize( + "fixture_name", + ["valid_excel_mfp_file", "valid_excel_mfp_file_with_commas"], +) +def test_mfp_to_yaml_success(request, fixture_name, tmp_path): + """Test that _mfp_to_yaml correctly processes a valid MFP Excel export.""" + valid_mfp_file = request.getfixturevalue(fixture_name) + yaml_output_path = tmp_path / "expedition.yaml" + start_date = "2023-10-20 01:00:00" + + _mfp_to_yaml(valid_mfp_file, start_date, yaml_output_path) + + # Ensure the YAML file was written + assert yaml_output_path.exists() + + # Load YAML and validate contents + data = Expedition.from_yaml(yaml_output_path) + + # 3 waypoints + 2 ports (departure & arrival) + assert len(data.schedule.waypoints) == 5 + assert isinstance(data.schedule.waypoints[0], Port) + assert isinstance(data.schedule.waypoints[-1], Port) + assert isinstance(data.schedule.waypoints[1], Waypoint) + + +@pytest.mark.parametrize( + "fixture_name,error,match", + [ + pytest.param( + "nonexistent_mfp_file", + FileNotFoundError, + r"File not found:", + id="FileNotFound", + ), + pytest.param( + "unsupported_extension_mfp_file", + RuntimeError, + "Could not read coordinates data from the provided file. Ensure it is an exported .xlsx file from MFP.", + id="UnsupportedExtension", + ), + pytest.param( + "invalid_mfp_file", + ValueError, + r"Error: Found columns .* but expected columns .*", + id="InvalidFile", + ), + pytest.param( + "missing_columns_mfp_file", + ValueError, + r"Error: Found columns .* but expected columns .*", + id="MissingColumns", + ), + ], +) +def test_mfp_to_yaml_exceptions(request, fixture_name, error, match, tmp_path): + """Test that _mfp_to_yaml raises an error when input file is not valid.""" + fixture = request.getfixturevalue(fixture_name) + yaml_output_path = tmp_path / "expedition.yaml" + start_date = "1998-05-01 01:00:00" + + with pytest.raises(error, match=match): + _mfp_to_yaml(fixture, start_date, yaml_output_path) + + +def test_mfp_to_yaml_extra_headers(unexpected_header_mfp_file, tmp_path): + """Test that _mfp_to_yaml prints a warning when extra columns are found.""" + yaml_output_path = tmp_path / "expedition.yaml" + start_date = "1998-05-01 01:00:00" + + with pytest.warns(UserWarning, match="Found additional unexpected columns.*"): + _mfp_to_yaml(unexpected_header_mfp_file, start_date, yaml_output_path) + + +def test_mfp_to_yaml_missing_ports_warning(missing_ports_mfp_file, tmp_path): + """Test that _mfp_to_yaml warns when departure or arrival ports are missing.""" + yaml_output_path = tmp_path / "expedition.yaml" + start_date = "1998-05-01 01:00:00" + + with pytest.warns( + UserWarning, + match="The MFP export is missing either a 'Departure Port' or 'Arrival Port'", + ): + _mfp_to_yaml(missing_ports_mfp_file, start_date, yaml_output_path) diff --git a/tests/cli/test_plan.py b/tests/cli/test_plan.py index 294592237..0b5eb51f3 100644 --- a/tests/cli/test_plan.py +++ b/tests/cli/test_plan.py @@ -17,12 +17,15 @@ SensorConfig, Waypoint, ) -from virtualship.utils import EXPEDITION, get_example_expedition +from virtualship.models.expedition import Port +from virtualship.utils import EXPEDITION, _get_example_expedition NEW_SPEED = "8.0" NEW_LAT = "0.015" NEW_LON = "0.015" +# TODO: new test that the check that there's Ports works + def _make_expedition( tmpdir: Path, @@ -32,9 +35,9 @@ def _make_expedition( """Write a minimal expedition YAML.""" if instruments_config is None: instruments_config = InstrumentsConfig.model_validate( - yaml.safe_load(get_example_expedition()).get("instruments_config") + yaml.safe_load(_get_example_expedition()).get("instruments_config") ) - ship_config = yaml.safe_load(get_example_expedition()).get("ship_config") + ship_config = yaml.safe_load(_get_example_expedition()).get("ship_config") Expedition( schedule=Schedule(waypoints=waypoints), instruments_config=instruments_config, @@ -74,6 +77,10 @@ async def _expand_instrument_configs( async def test_UI_changes(tmp_path): """Test making changes to UI inputs and saving to YAML (simulated botton presses and typing inputs).""" waypoints = [ + Port( + location=None, + time=None, + ), Waypoint( location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0), @@ -89,6 +96,10 @@ async def test_UI_changes(tmp_path): time=datetime(2022, 1, 1, 2, 0, 0), instrument=["CTD"], ), + Port( + location=None, + time=None, + ), ] _make_expedition(tmp_path, waypoints) @@ -123,18 +134,18 @@ async def test_UI_changes(tmp_path): wp_collapsible.collapsed = False await pilot.pause() lat_input, lon_input = ( - wp_collapsible.query_one("#wp1_lat", Input), - wp_collapsible.query_one("#wp1_lon", Input), + wp_collapsible.query_one("#wp2_lat", Input), + wp_collapsible.query_one("#wp2_lon", Input), ) await simulate_input(pilot, lat_input, NEW_LAT) await simulate_input(pilot, lon_input, NEW_LON) # toggle CTD on first waypoint - await pilot.click("#wp0_CTD") + await pilot.click("#wp1_CTD") await pilot.pause(0.1) # toggle XBT on first waypoint - await pilot.click("#wp0_XBT") + await pilot.click("#wp1_XBT") await pilot.pause(0.1) # re-collapse widget editors to make save button visible on screen @@ -149,10 +160,11 @@ async def test_UI_changes(tmp_path): await pilot.pause(0.5) # verify success notification received in UI (also useful for displaying potential debugging messages) - plan_screen.notify.assert_called_once_with( - "Changes saved successfully", - severity="information", - timeout=20, + 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 ) # verify changes to speed, lat, lon in saved YAML @@ -175,6 +187,7 @@ async def test_UI_changes(tmp_path): 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.""" waypoints = [ + Port(location=None, time=None), Waypoint( location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0), @@ -182,6 +195,7 @@ async def test_UI_opens_with_null_time_and_instrument(tmp_path): ), Waypoint(location=Location(0.01, 0.01), time=None, instrument=None), Waypoint(location=Location(0.02, 0.02), time=None, instrument=None), + Port(location=None, time=None), ] _make_expedition(tmp_path, waypoints) @@ -211,6 +225,7 @@ async def test_sensor_toggle_saved_to_yaml(tmp_path): _make_expedition( tmp_path, [ + Port(location=None, time=None), Waypoint( location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0), @@ -221,6 +236,7 @@ async def test_sensor_toggle_saved_to_yaml(tmp_path): time=datetime(2022, 1, 1, 1, 0, 0), instrument=None, ), + Port(location=None, time=None), ], ) @@ -240,8 +256,11 @@ async def test_sensor_toggle_saved_to_yaml(tmp_path): await pilot.click(plan_screen.query_one("#save_button", Button)) await pilot.pause(0.5) - plan_screen.notify.assert_called_once_with( - "Changes saved successfully", severity="information", timeout=20 + 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: @@ -257,6 +276,7 @@ async def test_deselecting_all_sensors_on_active_instrument_blocks_save(tmp_path _make_expedition( tmp_path, [ + Port(location=None, time=None), Waypoint( location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0), @@ -267,6 +287,7 @@ async def test_deselecting_all_sensors_on_active_instrument_blocks_save(tmp_path time=datetime(2022, 1, 1, 1, 0, 0), instrument=None, ), + Port(location=None, time=None), ], ) @@ -300,6 +321,7 @@ async def test_deselecting_all_sensors_on_inactive_instrument(tmp_path): _make_expedition( tmp_path, [ + Port(location=None, time=None), Waypoint( location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0), @@ -310,6 +332,7 @@ async def test_deselecting_all_sensors_on_inactive_instrument(tmp_path): time=datetime(2022, 1, 1, 1, 0, 0), instrument=None, ), + Port(location=None, time=None), ], ) @@ -331,8 +354,11 @@ async def test_deselecting_all_sensors_on_inactive_instrument(tmp_path): await pilot.click(plan_screen.query_one("#save_button", Button)) await pilot.pause(0.5) - plan_screen.notify.assert_called_once_with( - "Changes saved successfully", severity="information", timeout=20 + 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 ) @@ -346,12 +372,13 @@ async def test_sensor_initial_state_reflects_config(tmp_path): sensors=[SensorConfig(sensor_type=SensorType.TEMPERATURE)], ) instruments_config = InstrumentsConfig.model_validate( - yaml.safe_load(get_example_expedition()).get("instruments_config") + yaml.safe_load(_get_example_expedition()).get("instruments_config") ) instruments_config.ctd_config = ctd_config _make_expedition( tmp_path, [ + Port(location=None, time=None), Waypoint( location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0), @@ -362,6 +389,7 @@ async def test_sensor_initial_state_reflects_config(tmp_path): time=datetime(2022, 1, 1, 1, 0, 0), instrument=None, ), + Port(location=None, time=None), ], instruments_config, ) diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py index 0b6978616..2124eb52b 100644 --- a/tests/cli/test_run.py +++ b/tests/cli/test_run.py @@ -8,7 +8,7 @@ ScheduleOk, ) from virtualship.instruments.types import InstrumentType -from virtualship.utils import EXPEDITION, EXPEDITION_IDENTIFIER, get_example_expedition +from virtualship.utils import EXPEDITION, EXPEDITION_IDENTIFIER, _get_example_expedition def _simulate_schedule(projection, expedition): @@ -47,7 +47,7 @@ def test_run(tmp_path, monkeypatch): expedition_dir = tmp_path / "expedition_dir" expedition_dir.mkdir() - (expedition_dir / EXPEDITION).write_text(get_example_expedition()) + (expedition_dir / EXPEDITION).write_text(_get_example_expedition()) monkeypatch.setattr("virtualship.cli._run.simulate_schedule", _simulate_schedule) diff --git a/tests/expedition/expedition_dir/expedition.yaml b/tests/expedition/expedition_dir/expedition.yaml index 6392076b3..630d73428 100644 --- a/tests/expedition/expedition_dir/expedition.yaml +++ b/tests/expedition/expedition_dir/expedition.yaml @@ -2,23 +2,31 @@ # schedule: waypoints: + # Port of Departure + - location: + latitude: 0 + longitude: 0 + time: 2023-01-01 00:00:00 + # Waypoint 1 - instrument: - CTD location: latitude: 0 longitude: 0 - time: 2023-01-01 00:00:00 + time: 2023-01-02 00:00:00 + # Waypoint 2 - instrument: - DRIFTER - ARGO_FLOAT location: latitude: 0.01 longitude: 0.01 - time: 2023-01-02 00:00:00 - - location: # empty waypoint + time: 2023-01-03 00:00:00 + # Port of Arrival + - location: latitude: 0.02 longitude: 0.01 - time: 2023-01-02 03:00:00 + time: 2023-01-04 03:00:00 instruments_config: adcp_config: num_bins: 40 diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index 4bde12bdd..7c919f41c 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -7,6 +7,7 @@ import pyproj import pytest import xarray as xr +import yaml from virtualship.errors import InstrumentsConfigError, ScheduleError from virtualship.models import ( @@ -16,10 +17,11 @@ Waypoint, _InstrumentConfigMixin, ) +from virtualship.models.expedition import Port from virtualship.utils import ( EXPEDITION, + _get_example_expedition, _get_expedition, - get_example_expedition, ) projection = pyproj.Geod(ellps="WGS84") @@ -27,27 +29,19 @@ expedition_dir = Path("expedition_dir") -def test_import_export_expedition(tmpdir) -> None: - out_path = tmpdir.join(EXPEDITION) +@pytest.fixture +def base_expedition(): + """Shared expedition instance loaded directly from expedition_dir.""" + return _get_expedition(expedition_dir) - # arbitrary time for testing - base_time = datetime.strptime("1950-01-01", "%Y-%m-%d") - schedule = Schedule( - waypoints=[ - Waypoint(location=Location(0, 0), time=base_time, instrument=None), - Waypoint( - location=Location(1, 1), - time=base_time + timedelta(hours=1), - instrument=None, - ), - ] - ) - get_expedition = _get_expedition(expedition_dir) +def test_import_export_expedition(tmpdir, base_expedition) -> None: + out_path = tmpdir.join(EXPEDITION) + expedition = Expedition( - schedule=schedule, - instruments_config=get_expedition.instruments_config, - ship_config=get_expedition.ship_config, + schedule=base_expedition.schedule, + instruments_config=base_expedition.instruments_config, + ship_config=base_expedition.ship_config, ) expedition.to_yaml(out_path) @@ -55,56 +49,36 @@ def test_import_export_expedition(tmpdir) -> None: assert expedition == expedition2 -def test_verify_schedule() -> None: - schedule = Schedule( - waypoints=[ - Waypoint( - location=Location(0, 0), - time=datetime(2022, 1, 1, 1, 0, 0), - instrument=[], - ), - Waypoint( - location=Location(1, 0), - time=datetime(2022, 1, 2, 1, 0, 0), - instrument=[], - ), - ] +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, ) - ship_speed_knots = _get_expedition(expedition_dir).ship_config.ship_speed_knots - instruments_config = _get_expedition(expedition_dir).instruments_config - schedule.verify(ship_speed_knots, instruments_config, ignore_land_test=True) + assert schedule._verified, ( + "Schedule should be marked as verified after successful verification." + ) -def test_get_instruments() -> None: - get_expedition = _get_expedition(expedition_dir) - schedule = Schedule( - waypoints=[ - Waypoint(location=Location(0, 0), instrument=["CTD"]), - Waypoint(location=Location(1, 0), instrument=["XBT", "ARGO_FLOAT"]), - Waypoint(location=Location(1, 0), instrument=["CTD"]), - ] - ) +def test_get_instruments(base_expedition) -> None: expedition = Expedition( - schedule=schedule, - instruments_config=get_expedition.instruments_config, - ship_config=get_expedition.ship_config, - ) - assert ( - set(instrument.name for instrument in expedition.get_instruments()) - == { - "CTD", - "UNDERWATER_ST", # not added above but underway instruments are auto present from instruments_config in expedition_dir/expedition.yaml - "ADCP", # as above - "ARGO_FLOAT", - "XBT", - } + schedule=base_expedition.schedule, + instruments_config=base_expedition.instruments_config, + ship_config=base_expedition.ship_config, ) + assert set(instrument.name for instrument in expedition.get_instruments()) == { + "CTD", + "UNDERWATER_ST", + "ADCP", + "ARGO_FLOAT", + "DRIFTER", + } -def test_verify_on_land(): +def test_verify_on_land(base_expedition): """Test that schedule verification raises error for waypoints on land (0.0 m bathymetry).""" - # bathymetry fieldset with NaNs at specific locations lat = np.array([0, 1.0, 2.0]) lon = np.array([0, 1.0, 2.0]) bathymetry = np.array( @@ -116,9 +90,7 @@ def test_verify_on_land(): ) ds_bathymetry = xr.Dataset( - { - "deptho": (("lat", "lon"), bathymetry), - }, + {"deptho": (("lat", "lon"), bathymetry)}, coords={ "lon": (("lon"), lon, {"units": "degrees_east"}), "lat": (("lat"), lat, {"units": "degrees_north"}), @@ -128,25 +100,17 @@ def test_verify_on_land(): ds_fset = parcels.convert.copernicusmarine_to_sgrid( fields={"bathymetry": ds_bathymetry["deptho"]}, ) - bathymetry_fieldset = parcels.FieldSet.from_sgrid_conventions(ds_fset) - # waypoints placed in NaN bathy cells - waypoints = [ - Waypoint( - location=Location(0.0, 1.0), time=datetime(2022, 1, 1, 1, 0, 0) - ), # NaN cell - Waypoint( - location=Location(1.0, 2.0), time=datetime(2022, 1, 2, 1, 0, 0) - ), # NaN cell - Waypoint( - location=Location(2.0, 0.0), time=datetime(2022, 1, 3, 1, 0, 0) - ), # NaN cell - ] - - schedule = Schedule(waypoints=waypoints) - ship_speed_knots = _get_expedition(expedition_dir).ship_config.ship_speed_knots - instruments_config = _get_expedition(expedition_dir).instruments_config + schedule = Schedule( + waypoints=[ + Port(location=Location(0, 0), time=datetime(2022, 1, 1, 1, 0, 0)), + Waypoint(location=Location(0.0, 1.0), time=datetime(2022, 1, 2, 1, 0, 0)), + Waypoint(location=Location(1.0, 2.0), time=datetime(2022, 1, 3, 1, 0, 0)), + Waypoint(location=Location(2.0, 0.0), time=datetime(2022, 1, 4, 1, 0, 0)), + Port(location=Location(1, 0), time=datetime(2022, 1, 5, 1, 0, 0)), + ] + ) with patch( "virtualship.models.expedition._get_bathy_data", @@ -157,79 +121,81 @@ def test_verify_on_land(): match=r"The following waypoint\(s\) throw\(s\) error\(s\):", ): schedule.verify( - ship_speed_knots, - instruments_config, + base_expedition.ship_config.ship_speed_knots, + base_expedition.instruments_config, ignore_land_test=False, from_data=None, ) @pytest.mark.parametrize( - "schedule,error,match", + "waypoints,error,match", [ pytest.param( - Schedule(waypoints=[]), + [Waypoint(location=Location(0, 0))], ScheduleError, - "At least one waypoint must be provided.", + r"First and last waypoints must be Ports \(of arrival/departure\)\.", + id="NoPorts", + ), + pytest.param( + [ + Port(location=Location(0, 0)), + Port(location=Location(1, 0)), + ], + ScheduleError, + "At least one non-port waypoint must be provided.", id="NoWaypoints", ), pytest.param( - Schedule( - waypoints=[ - Waypoint(location=Location(0, 0)), - Waypoint( - location=Location(1, 0), time=datetime(2022, 1, 1, 1, 0, 0) - ), - ] - ), + [ + Port(location=Location(0, 0)), + Waypoint(location=Location(0, 0)), + Waypoint(location=Location(1, 0), time=datetime(2022, 1, 1, 1, 0, 0)), + Port(location=Location(1, 0)), + ], ScheduleError, - "First waypoint must have a specified time.", + "Waypoint 1 must have a specified time.", id="FirstWaypointHasTime", ), pytest.param( - Schedule( - waypoints=[ - Waypoint( - location=Location(0, 0), time=datetime(2022, 1, 2, 1, 0, 0) - ), - Waypoint(location=Location(0, 0)), - Waypoint( - location=Location(1, 0), time=datetime(2022, 1, 1, 1, 0, 0) - ), - ] - ), + [ + Port(location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0)), + Waypoint(location=Location(0, 0), time=datetime(2022, 1, 2, 1, 0, 0)), + Waypoint(location=Location(0, 0)), + Waypoint(location=Location(1, 0), time=datetime(2022, 1, 1, 1, 0, 0)), + Port(location=Location(1, 0), time=datetime(2022, 1, 3, 0, 0, 0)), + ], ScheduleError, - "Waypoint\\(s\\) : each waypoint should be timed after all previous waypoints", + r"Waypoint\(s\).*?: each waypoint should be timed after all previous waypoints", id="SequentialWaypoints", ), pytest.param( - Schedule( - waypoints=[ - Waypoint( - location=Location(0, 0), - time=datetime(2022, 1, 1, 1, 0, 0), - instrument=[], - ), - Waypoint( - location=Location(1, 0), - time=datetime(2022, 1, 1, 1, 1, 0), - instrument=[], - ), - ] - ), + [ + Port(location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0)), + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 1, 0, 0), + instrument=[], + ), + Waypoint( + location=Location(1, 0), + time=datetime(2022, 1, 1, 1, 1, 0), + instrument=[], + ), + Port(location=Location(1, 0), time=datetime(2022, 1, 2, 0, 0, 0)), + ], ScheduleError, - "Waypoint planning is not valid: would arrive too late at waypoint 2\\.", + r"Waypoint planning is not valid: would arrive too late at waypoint 2\.", id="NotEnoughTime", ), ], ) -def test_verify_schedule_errors(schedule: Schedule, error, match) -> None: - expedition = _get_expedition(expedition_dir) - +def test_verify_schedule_errors(base_expedition, waypoints: list, error, match) -> None: with pytest.raises(error, match=match): + schedule = Schedule(waypoints=waypoints) schedule.verify( - expedition.ship_config.ship_speed_knots, - expedition.instruments_config, + base_expedition.ship_config.ship_speed_knots, + base_expedition.instruments_config, ignore_land_test=True, ) @@ -237,116 +203,66 @@ def test_verify_schedule_errors(schedule: Schedule, error, match) -> None: @pytest.fixture def expedition(tmp_file): with open(tmp_file, "w") as file: - file.write(get_example_expedition()) + file.write(_get_example_expedition()) return Expedition.from_yaml(tmp_file) @pytest.fixture def expedition_no_xbt(expedition): for waypoint in expedition.schedule.waypoints: - if waypoint.instrument and any( - instrument.name == "XBT" for instrument in waypoint.instrument - ): - waypoint.instrument = [ - instrument - for instrument in waypoint.instrument - if instrument.name != "XBT" - ] - + instruments = getattr(waypoint, "instrument", None) + if instruments and any(instrument.name == "XBT" for instrument in instruments): + waypoint.instrument = [inst for inst in instruments if inst.name != "XBT"] return expedition -@pytest.fixture -def instruments_config_no_xbt(expedition): - delattr(expedition.instruments_config, "xbt_config") - return expedition.instruments_config - - -@pytest.fixture -def instruments_config_no_ctd(expedition): - delattr(expedition.instruments_config, "ctd_config") - return expedition.instruments_config - - -@pytest.fixture -def instruments_config_no_argo_float(expedition): - delattr(expedition.instruments_config, "argo_float_config") - return expedition.instruments_config - - -@pytest.fixture -def instruments_config_no_drifter(expedition): - delattr(expedition.instruments_config, "drifter_config") - return expedition.instruments_config - - -@pytest.fixture -def instruments_config_no_adcp(expedition): - delattr(expedition.instruments_config, "adcp_config") - return expedition.instruments_config - - -@pytest.fixture -def instruments_config_no_underwater_st(expedition): - delattr(expedition.instruments_config, "ship_underwater_st_config") - return expedition.instruments_config - - -def test_verify_instruments_config(expedition) -> None: - expedition.instruments_config.verify(expedition) - - -def test_verify_instruments_config_no_instrument(expedition, expedition_no_xbt) -> None: - expedition.instruments_config.verify(expedition_no_xbt) - - -@pytest.mark.parametrize( - "instruments_config_fixture,error,match", - [ - pytest.param( - "instruments_config_no_xbt", - InstrumentsConfigError, +@pytest.fixture( + params=[ + ( + "xbt_config", "Expedition includes instrument 'XBT', but instruments_config does not provide configuration for it.", - id="InstrumentsConfigNoXBT", ), - pytest.param( - "instruments_config_no_ctd", - InstrumentsConfigError, + ( + "ctd_config", "Expedition includes instrument 'CTD', but instruments_config does not provide configuration for it.", - id="InstrumentsConfigNoCTD", ), - pytest.param( - "instruments_config_no_argo_float", - InstrumentsConfigError, + ( + "argo_float_config", "Expedition includes instrument 'ARGO_FLOAT', but instruments_config does not provide configuration for it.", - id="InstrumentsConfigNoARGO_FLOAT", ), - pytest.param( - "instruments_config_no_drifter", - InstrumentsConfigError, + ( + "drifter_config", "Expedition includes instrument 'DRIFTER', but instruments_config does not provide configuration for it.", - id="InstrumentsConfigNoDRIFTER", ), - pytest.param( - "instruments_config_no_adcp", - InstrumentsConfigError, + ( + "adcp_config", r"Underway instrument config attribute\(s\) are missing from YAML\. Must be Config object or None\.", - id="InstrumentsConfigNoADCP", ), - pytest.param( - "instruments_config_no_underwater_st", - InstrumentsConfigError, + ( + "ship_underwater_st_config", r"Underway instrument config attribute\(s\) are missing from YAML\. Must be Config object or None\.", - id="InstrumentsConfigNoUNDERWATER_ST", ), - ], + ] ) +def missing_instrument_config(request, expedition): + attr_name, error_match = request.param + delattr(expedition.instruments_config, attr_name) + return expedition.instruments_config, error_match + + +def test_verify_instruments_config(expedition) -> None: + expedition.instruments_config.verify(expedition) + + +def test_verify_instruments_config_no_instrument(expedition, expedition_no_xbt) -> None: + expedition.instruments_config.verify(expedition_no_xbt) + + def test_verify_instruments_config_errors( - request, expedition, instruments_config_fixture, error, match + expedition, missing_instrument_config ) -> None: - instruments_config = request.getfixturevalue(instruments_config_fixture) - - with pytest.raises(error, match=match): + instruments_config, match = missing_instrument_config + with pytest.raises(InstrumentsConfigError, match=match): instruments_config.verify(expedition) @@ -354,20 +270,117 @@ def test_all_instrument_configs_use_mixin(expedition): """Every registered instrument config must inherit _InstrumentConfigMixin and define the required ClassVars.""" instrument_configs = [ iconfig - for _, iconfig in expedition.instruments_config.__dict__.items() + for iconfig in expedition.instruments_config.__dict__.values() if iconfig ] for iconfig in instrument_configs: - assert issubclass(iconfig.__class__, _InstrumentConfigMixin), ( - f"{iconfig.__class__.__name__} does not inherit _InstrumentConfigMixin" + cls = iconfig.__class__ + assert issubclass(cls, _InstrumentConfigMixin), ( + f"{cls.__name__} does not inherit _InstrumentConfigMixin" ) - assert "_instrument_type" in iconfig.__class__.__dict__, ( - f"{iconfig.__class__.__name__} does not define _instrument_type" + assert "_instrument_type" in cls.__dict__, ( + f"{cls.__name__} does not define _instrument_type" ) - assert "_instrument_name" in iconfig.__class__.__dict__, ( - f"{iconfig.__class__.__name__} does not define _instrument_name" + assert "_instrument_name" in cls.__dict__, ( + f"{cls.__name__} does not define _instrument_name" ) - assert iconfig.__class__._instrument_type == iconfig._instrument_type, ( - f"{iconfig.__class__.__name__}._instrument_type does not match its registered InstrumentType" + assert cls._instrument_type == iconfig._instrument_type, ( + f"{cls.__name__}._instrument_type mismatch" ) + + +def test_waypoint_yaml_lines(base_expedition) -> None: + """Each full waypoint entry in the raw YAML dump should start with '- instrument:', whereas Port waypoints should start with just '- location:'.""" + schedule = base_expedition.schedule + raw = yaml.dump( + { + "schedule": { + "waypoints": [wp.model_dump(by_alias=True) for wp in schedule.waypoints] + } + }, + default_flow_style=False, + ) + + standard_lines = [ + line for line in raw.splitlines() if line.lstrip().startswith("- instrument:") + ] + port_lines = [ + line for line in raw.splitlines() if line.lstrip().startswith("- location:") + ] + + port_wps = [wp for wp in schedule.waypoints if isinstance(wp, Port)] + standard_wps = [wp for wp in schedule.waypoints if not isinstance(wp, Port)] + + assert len(port_wps) == 2, ( + "There should be exactly 2 Port waypoints (departure and arrival)." + ) + + assert len(port_lines) == len(port_wps), ( + f"Expected {len(port_wps)} lines starting with '- location:' in the YAML dump, " + f"got {len(port_lines)}. The Port/Waypoint field order or terminology may have changed. " + "Note this can have implications for the placement of port/waypoint number comments in Expedition.to_yaml()." + ) + + assert len(standard_lines) == len(standard_wps), ( + f"Expected {len(standard_wps)} lines starting with '- instrument:' in the YAML dump, " + f"got {len(standard_lines)}. The Waypoint field order or terminology may have changed. " + "Note this can have implications for the placement of waypoint number comments in Expedition.to_yaml()." + ) + + +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") + schedule = Schedule( + waypoints=[ + Port(location=Location(None, None), time=None), + Waypoint(location=Location(1, 1), time=base_time + timedelta(hours=1)), + Waypoint(location=Location(2, 2), time=base_time + timedelta(hours=2)), + Port(location=Location(None, None), time=None), + ] + ) + expedition = Expedition( + schedule=schedule, + instruments_config=base_expedition.instruments_config, + ship_config=base_expedition.ship_config, + ) + + wps_in_use = expedition.schedule._get_wps_in_use() + assert len(wps_in_use) == 2 # placeholder waypoints should be removed + assert all(isinstance(wp, Waypoint) for wp in wps_in_use) + + +def test_wps_in_use_asymmetric_placeholder_ports(): + """Only the inactive side (departure and/or arrival) should be excluded, not both.""" + base_time = datetime.strptime("1950-01-01", "%Y-%m-%d") + wp1 = Waypoint(location=Location(1, 1), time=base_time + timedelta(hours=1)) + active_arrival = Port(location=Location(2, 2), time=base_time + timedelta(hours=2)) + + # inactive departure, active arrival + schedule = Schedule(waypoints=[Port(location=None, time=None), wp1, active_arrival]) + wps_in_use = schedule._get_wps_in_use() + assert wps_in_use == [wp1, active_arrival] + + # active departure, inactive arrival + active_departure = Port(location=Location(0, 0), time=base_time) + schedule = Schedule( + waypoints=[active_departure, wp1, Port(location=None, time=None)] + ) + wps_in_use = schedule._get_wps_in_use() + assert wps_in_use == [active_departure, wp1] + + +@pytest.mark.parametrize( + "location, time, expected", + [ + (Location(0, 0), datetime(2024, 1, 1), True), + (None, datetime(2024, 1, 1), False), + (Location(0, 0), None, False), + (None, None, False), + (Location(None, None), datetime(2024, 1, 1), False), + ], +) +def test_port_is_in_use(location, time, expected): + """A Port is only 'in use' when it has both a fully-specified location and a time.""" + assert Port(location=location, time=time).is_in_use is expected diff --git a/tests/expedition/test_simulate_schedule.py b/tests/expedition/test_simulate_schedule.py index 35dfbdea6..f33e3b523 100644 --- a/tests/expedition/test_simulate_schedule.py +++ b/tests/expedition/test_simulate_schedule.py @@ -2,13 +2,14 @@ import numpy as np import pyproj +import pytest from virtualship.expedition.simulate_schedule import ( ScheduleOk, - ScheduleProblem, simulate_schedule, ) from virtualship.models import Expedition, Location, Schedule, Waypoint +from virtualship.models.expedition import Port def test_simulate_schedule_feasible() -> None: @@ -20,35 +21,20 @@ def test_simulate_schedule_feasible() -> None: expedition.ship_config.ship_speed_knots = 10.0 expedition.schedule = Schedule( waypoints=[ + Port(location=None, time=None), Waypoint(location=Location(0, 0), time=base_time), Waypoint(location=Location(0.01, 0), time=base_time + timedelta(days=1)), + Port(location=None, time=None), ] ) + # assume the schedule has been verified + expedition.schedule._verified = True result = simulate_schedule(projection, expedition) assert isinstance(result, ScheduleOk) -def test_simulate_schedule_too_far() -> None: - """Test schedule with two waypoints that are very far away and cannot be reached in time is not 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.ship_config.ship_speed_knots = 10.0 - expedition.schedule = Schedule( - waypoints=[ - Waypoint(location=Location(0, 0), time=base_time), - Waypoint(location=Location(1.0, 0), time=base_time + timedelta(minutes=1)), - ] - ) - - result = simulate_schedule(projection, expedition) - - assert isinstance(result, ScheduleProblem) - - 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( @@ -81,26 +67,34 @@ def test_ship_path_inside_domain() -> None: # waypoints with enough distance where curvature is clear expedition.schedule = Schedule( waypoints=[ + Port(location=None, time=None), Waypoint(location=wp1, time=base_time), Waypoint(location=wp2, time=base_time + timedelta(days=5)), Waypoint(location=wp3, time=base_time + timedelta(days=10)), Waypoint(location=wp4, time=base_time + timedelta(days=15)), + Port(location=None, time=None), ] ) # get waypoint domain bounds + wps_in_use = expedition.schedule._get_wps_in_use() + wp_max_lat, wp_min_lat, wp_max_lon, wp_min_lon = ( - max(wp.location.lat for wp in expedition.schedule.waypoints), - min(wp.location.lat for wp in expedition.schedule.waypoints), - max(wp.location.lon for wp in expedition.schedule.waypoints), - min(wp.location.lon for wp in expedition.schedule.waypoints), + max(wp.location.lat for wp in wps_in_use), + min(wp.location.lat for wp in wps_in_use), + max(wp.location.lon for wp in wps_in_use), + min(wp.location.lon for wp in wps_in_use), ) + # assume the schedule has been verified + expedition.schedule._verified = True + result = simulate_schedule(projection, expedition) assert isinstance(result, ScheduleOk) # adcp measurements path adcp_measurements = result.measurements_to_simulate.adcps + adcp_lats = [m.location.lat for m in adcp_measurements] adcp_lons = [m.location.lon for m in adcp_measurements] @@ -122,3 +116,27 @@ def test_ship_path_inside_domain() -> None: assert np.isclose(adcp_min_lat, wp1.lat, atol=0.1) assert np.isclose(adcp_max_lon, wp4.lon, atol=0.1) assert np.isclose(adcp_min_lon, wp3.lon, atol=0.1) + + +def test_does_not_simulate_unverified(): + """Test that simulating an unverified schedule raises an error.""" + 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.ship_config.ship_speed_knots = 10.0 + expedition.schedule = Schedule( + waypoints=[ + Port(location=None, time=None), + Waypoint(location=Location(0, 0), time=base_time), + Waypoint(location=Location(0.01, 0), time=base_time + timedelta(days=1)), + Port(location=None, time=None), + ] + ) + + expedition.schedule._verified = False + + with pytest.raises( + AssertionError, match=r"Schedule must be verified before simulation." + ): + simulate_schedule(projection, expedition) diff --git a/tests/instruments/test_adcp.py b/tests/instruments/test_adcp.py index ab13f99cc..d5792094b 100644 --- a/tests/instruments/test_adcp.py +++ b/tests/instruments/test_adcp.py @@ -32,15 +32,20 @@ def adcp_expedition(): """Minimal Expedition for ADCPInstrument instantiation.""" + class DummySchedule: + waypoints: ClassVar[list] = [ + Waypoint( + location=Location(1, 2), + time=BASE_TIME, + instrument=[], + ), + ] + + def _get_wps_in_use(self): + return self.waypoints + class DummyExpedition: - class schedule: - waypoints: ClassVar[list] = [ - Waypoint( - location=Location(1, 2), - time=BASE_TIME, - instrument=InstrumentType.ADCP, - ), - ] + schedule = DummySchedule() instruments_config = InstrumentsConfig( adcp_config=ADCPConfig( diff --git a/tests/instruments/test_argo_float.py b/tests/instruments/test_argo_float.py index 2917a8757..1f519816c 100644 --- a/tests/instruments/test_argo_float.py +++ b/tests/instruments/test_argo_float.py @@ -3,6 +3,7 @@ import contextlib import io from datetime import datetime, timedelta +from typing import ClassVar import numpy as np import parcels @@ -128,11 +129,20 @@ def create_argo_float(waypoint): def create_dummy_expedition(sensors, lifetime=timedelta(days=1), location=(1, 2)): """Create a DummyExpedition class with specified sensors and parameters.""" + class DummySchedule: + waypoints: ClassVar[list] = [ + Waypoint( + location=Location(*location), + time=BASE_TIME, + instrument=[InstrumentType.ARGO_FLOAT], + ), + ] + + def _get_wps_in_use(self): + return self.waypoints + class DummyExpedition: - class schedule: - waypoints: list[Waypoint] = [ # noqa: RUF012 - Waypoint(location=Location(*location), time=BASE_TIME) - ] + schedule = DummySchedule() instruments_config = InstrumentsConfig( argo_float_config=ArgoFloatConfig( diff --git a/tests/instruments/test_base.py b/tests/instruments/test_base.py index b69554912..44b4f13af 100644 --- a/tests/instruments/test_base.py +++ b/tests/instruments/test_base.py @@ -1,4 +1,5 @@ from dataclasses import dataclass +from datetime import datetime from typing import ClassVar from unittest.mock import MagicMock, patch @@ -11,6 +12,7 @@ from virtualship.instruments.base import ( FetchSpec, Instrument, + SpatialBounds, UnderwayCoordinates, UnderwayInstrument, ) @@ -24,6 +26,30 @@ # ============================================================================= +@pytest.fixture() +def mock_waypoints(): + """Shared fixture providing mock Waypoint objects.""" + wp1 = MagicMock() + wp1.location.latitude = 10.0 + wp1.location.longitude = -20.0 + wp1.time = datetime(2026, 1, 1, 12, 0) + + wp2 = MagicMock() + wp2.location.latitude = 15.0 + wp2.location.longitude = -15.0 + wp2.time = datetime(2026, 1, 5, 12, 0) + + return [wp1, wp2] + + +@pytest.fixture() +def mock_expedition(mock_waypoints): + """Shared fixture providing a mock Expedition initialized with waypoints.""" + expedition = MagicMock() + expedition.schedule._get_wps_in_use.return_value = mock_waypoints + return expedition + + @pytest.fixture() def fieldset(): """Minimal Parcels FieldSet containing a temperature field.""" @@ -49,7 +75,7 @@ def fieldset(): @pytest.fixture() def pset(fieldset): - """Minimal ParticleSet initialized with a custom Particle class and the fieldset fixture.""" + """Minimal ParticleSet initialized with a custom Particle class and fieldset fixture.""" SampleParticle = parcels.Particle.add_variable(parcels.Variable("temperature")) t1 = np.datetime64("2024-01-01T00:00:00") @@ -59,18 +85,16 @@ def pset(fieldset): # ============================================================================= -# Instrument base class testing +# SpatialBounds & FetchSpec Tests # ============================================================================= def test_FetchSpec(): fetch_spec = FetchSpec() - # test that default values are set assert fetch_spec.latlon_buffer is not None assert fetch_spec.time_buffer is not None - # test setting values (in new instance) and that original is unchanged in memory fetch_spec2 = FetchSpec(latlon_buffer=0.5, time_buffer=1.0) assert fetch_spec2.latlon_buffer == 0.5 assert fetch_spec2.time_buffer == 1.0 @@ -78,6 +102,42 @@ def test_FetchSpec(): assert fetch_spec.latlon_buffer != fetch_spec2.latlon_buffer +def test_spatial_bounds_from_waypoints(mock_waypoints): + """Verify bounds calculations and 1-day time buffer addition.""" + with patch( + "virtualship.instruments.base._get_waypoint_latlons", + return_value=([10.0, 15.0], [-20.0, -15.0]), + ): + bounds = SpatialBounds.from_waypoints(mock_waypoints) + + assert bounds.min_lat == 10.0 + assert bounds.max_lat == 15.0 + assert bounds.min_lon == -20.0 + assert bounds.max_lon == -15.0 + assert bounds.min_time == datetime(2026, 1, 1, 12, 0) + assert bounds.max_time == datetime(2026, 1, 6, 12, 0) # +1 day applied + + +def test_spatial_bounds_with_buffer(): + """Verify buffer padding returns correct order (min_lon, max_lon, min_lat, max_lat).""" + bounds = SpatialBounds( + min_lat=-10.0, + max_lat=10.0, + min_lon=-50.0, + max_lon=-40.0, + min_time=datetime(2026, 1, 1), + max_time=datetime(2026, 1, 2), + ) + + assert bounds.with_buffer(0.0) == (-50.0, -40.0, -10.0, 10.0) + assert bounds.with_buffer(0.5) == (-50.5, -39.5, -10.5, 10.5) + + +# ============================================================================= +# Instrument Base Class Tests +# ============================================================================= + + def test_all_instruments_have_instrument_class(): for instrument in InstrumentType: instrument_class = get_instrument_class(instrument) @@ -89,18 +149,18 @@ class DummyInstrument(Instrument): sensor_kernels = {} # noqa - def simulate(self, data_dir, measurements, out_path): + def simulate(self, measurements, out_path): """Dummy simulate implementation for test.""" self.simulate_called = True @property def instrument_type(self) -> InstrumentType: - """Return a valid InstrumentType for the test.""" + """Return a valid InstrumentType for testing.""" return InstrumentType.CTD class _FakeFieldSet: - """Minimal fieldset.""" + """Minimal fieldset structure.""" def __init__(self, **fields): for name, value in fields.items(): @@ -112,14 +172,10 @@ def to_windowed_arrays(self): return self -def test_load_input_data(): +def test_load_input_data(mock_expedition): """Test Instrument.load_input_data with mocks.""" - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - dummy = DummyInstrument( - expedition=MagicMock(schedule=MagicMock(waypoints=[mock_waypoint])), + expedition=mock_expedition, variables={"A": "a"}, add_bathymetry=False, verbose_progress=False, @@ -151,14 +207,10 @@ def test_load_input_data(): assert fieldset == fake_fieldset -def test_gets_uv_vectorfield_when_u_and_v_present(): +def test_gets_uv_vectorfield_when_u_and_v_present(mock_expedition): """load_input_data creates a 'UV' VectorField when U and V fields are present.""" - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - dummy = DummyInstrument( - expedition=MagicMock(schedule=MagicMock(waypoints=[mock_waypoint])), + expedition=mock_expedition, variables={"U": "uo", "V": "vo"}, add_bathymetry=False, verbose_progress=False, @@ -187,14 +239,9 @@ def test_gets_uv_vectorfield_when_u_and_v_present(): assert result.fields["UV"] is mock_uv -def test_execute_calls_simulate(monkeypatch): - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - mock_schedule = MagicMock() - mock_schedule.waypoints = [mock_waypoint] +def test_execute_calls_simulate(mock_expedition): dummy = DummyInstrument( - expedition=MagicMock(schedule=mock_schedule), + expedition=mock_expedition, variables={"A": "a"}, add_bathymetry=False, verbose_progress=True, @@ -205,16 +252,11 @@ def test_execute_calls_simulate(monkeypatch): dummy.simulate.assert_called_once() -def test_fetch_spec_applied_to_instrument(): +def test_fetch_spec_applied_to_instrument(mock_expedition): """FetchSpec values are correctly stored on the instrument.""" - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - mock_schedule = MagicMock() - mock_schedule.waypoints = [mock_waypoint] fetch_spec = FetchSpec(latlon_buffer=5.0, depth_min=-10.0) dummy = DummyInstrument( - expedition=MagicMock(schedule=mock_schedule), + expedition=mock_expedition, variables={"A": "a"}, add_bathymetry=False, verbose_progress=False, @@ -223,19 +265,14 @@ def test_fetch_spec_applied_to_instrument(): ) assert dummy.fetch_spec.latlon_buffer == 5.0 assert dummy.fetch_spec.depth_min == -10.0 - # unset values use dataclass defaults assert dummy.fetch_spec.time_buffer == 0.0 assert dummy.fetch_spec.depth_max is None -def test_via_tmp_ds_roundtrip(): +def test_via_tmp_ds_roundtrip(mock_expedition): """_via_tmp_ds writes to a tmp file and re-opens it.""" - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - with DummyInstrument( - expedition=MagicMock(schedule=MagicMock(waypoints=[mock_waypoint])), + expedition=mock_expedition, variables={"A": "a"}, add_bathymetry=False, verbose_progress=False, @@ -249,22 +286,16 @@ def test_via_tmp_ds_roundtrip(): assert isinstance(result, xr.Dataset) assert "temperature" in result - assert ( - result is not ds - ) # result is new object loaded from tmp file, not the original + assert result is not ds result.close() ds.close() -def test_instrument_context_manager(): - """Test that context manager cleans up temporary directories upon exit.""" - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - +def test_instrument_context_manager(mock_expedition): + """Test context manager cleanup of temporary directories.""" with DummyInstrument( - expedition=MagicMock(schedule=MagicMock(waypoints=[mock_waypoint])), + expedition=mock_expedition, variables={"A": "a"}, add_bathymetry=False, verbose_progress=False, @@ -279,17 +310,12 @@ def test_instrument_context_manager(): result.close() ds.close() - # outside 'with' block, tmp dirs should be cleared assert len(dummy._tmp_dirs) == 0 -def test_generate_fieldset_combines_fields(): - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - +def test_generate_fieldset_combines_fields(mock_expedition): dummy = DummyInstrument( - expedition=MagicMock(schedule=MagicMock(waypoints=[mock_waypoint])), + expedition=mock_expedition, variables={"A": "a", "B": "b"}, add_bathymetry=False, verbose_progress=False, @@ -313,14 +339,9 @@ def test_generate_fieldset_combines_fields(): fs_A.__add__.assert_called_once_with(fs_B) -def test_load_input_data_error(monkeypatch): - mock_waypoint = MagicMock() - mock_waypoint.location.latitude = 1.0 - mock_waypoint.location.longitude = 2.0 - mock_schedule = MagicMock() - mock_schedule.waypoints = [mock_waypoint] +def test_load_input_data_error(mock_expedition, monkeypatch): dummy = DummyInstrument( - expedition=MagicMock(schedule=mock_schedule), + expedition=mock_expedition, variables={"A": "a"}, add_bathymetry=False, verbose_progress=False, @@ -331,10 +352,10 @@ def test_load_input_data_error(monkeypatch): ) import virtualship.errors - try: + with pytest.raises( + virtualship.errors.CopernicusCatalogueError, match="Failed to load input data" + ): dummy.load_input_data() - except virtualship.errors.CopernicusCatalogueError as e: - assert "Failed to load input data" in str(e) def test_instrument_subclass_without_sensor_kernels_error(): @@ -364,6 +385,42 @@ def test_instrument_samples_initial_conditions(fieldset, pset): ) +def test_instrument_init_filters_out_placeholder_ports(mock_expedition, mock_waypoints): + """Verify Instrument init uses _get_wps_in_use to strip null ports.""" + null_port = MagicMock() + null_port.location.latitude = None + null_port.location.longitude = None + null_port.time = None + + # insert placeholder ports around the valid mock_waypoints + mock_expedition.schedule._get_wps_in_use.return_value = [ + null_port, + *mock_waypoints, + null_port, + ] + + with patch( + "virtualship.instruments.base._get_instr_relevant_wps", + return_value=mock_waypoints, + ) as mock_filter: + dummy = DummyInstrument( + expedition=mock_expedition, + variables={"A": "a"}, + add_bathymetry=False, + verbose_progress=False, + from_data=None, + ) + + mock_filter.assert_called_once_with( + mock_expedition.schedule._get_wps_in_use(), + dummy.instrument_type, + ) + + assert dummy.bounds.min_lat == 10.0 + assert dummy.bounds.max_lat == 15.0 + assert dummy.bounds.min_time == datetime(2026, 1, 1, 12, 0) + + # ============================================================================= # UnderwayInstrument intermediate class testing # ============================================================================= diff --git a/tests/instruments/test_ctd.py b/tests/instruments/test_ctd.py index 3e63a8bd2..cc8a4ad15 100644 --- a/tests/instruments/test_ctd.py +++ b/tests/instruments/test_ctd.py @@ -5,6 +5,7 @@ """ import datetime +from typing import ClassVar import numpy as np import parcels @@ -35,11 +36,20 @@ def create_dummy_expedition( ): """Create a DummyExpedition class with specified sensors and parameters.""" + class DummySchedule: + waypoints: ClassVar[list] = [ + Waypoint( + location=Location(*location), + time=BASE_TIME, + instrument=[InstrumentType.CTD], + ), + ] + + def _get_wps_in_use(self): + return self.waypoints + class DummyExpedition: - class schedule: - waypoints: list[Waypoint] = [ # noqa: RUF012 - Waypoint(location=Location(*location), time=BASE_TIME) - ] + schedule = DummySchedule() instruments_config = InstrumentsConfig( ctd_config=CTDConfig( diff --git a/tests/instruments/test_drifter.py b/tests/instruments/test_drifter.py index 21ac2afad..704c92160 100644 --- a/tests/instruments/test_drifter.py +++ b/tests/instruments/test_drifter.py @@ -1,6 +1,7 @@ """Test the simulation of drifters.""" import datetime +from typing import ClassVar import numpy as np import parcels @@ -34,11 +35,20 @@ def create_dummy_expedition( if sensors is None: sensors = [SensorConfig(sensor_type=SensorType.TEMPERATURE)] + class DummySchedule: + waypoints: ClassVar[list] = [ + Waypoint( + location=Location(*location), + time=BASE_TIME, + instrument=[InstrumentType.DRIFTER], + ), + ] + + def _get_wps_in_use(self): + return self.waypoints + class DummyExpedition: - class schedule: - waypoints: list[Waypoint] = [ # noqa: RUF012 - Waypoint(location=Location(*location), time=BASE_TIME) - ] + schedule = DummySchedule() instruments_config = InstrumentsConfig( drifter_config=DrifterConfig( diff --git a/tests/instruments/test_ship_underwater_st.py b/tests/instruments/test_ship_underwater_st.py index 1bf1689c2..f7e0e0a2e 100644 --- a/tests/instruments/test_ship_underwater_st.py +++ b/tests/instruments/test_ship_underwater_st.py @@ -30,15 +30,20 @@ def underwater_st_expedition(): """Minimal Expedition for Underwater_STInstrument instantiation.""" + class DummySchedule: + waypoints: ClassVar[list] = [ + Waypoint( + location=Location(1, 2), + time=BASE_TIME, + instrument=[], + ), + ] + + def _get_wps_in_use(self): + return self.waypoints + class DummyExpedition: - class schedule: - waypoints: ClassVar[list] = [ - Waypoint( - location=Location(1, 2), - time=BASE_TIME, - instrument=InstrumentType.UNDERWATER_ST, - ), - ] + schedule = DummySchedule() instruments_config = InstrumentsConfig( ship_underwater_st_config=ShipUnderwaterSTConfig( diff --git a/tests/instruments/test_xbt.py b/tests/instruments/test_xbt.py index d55ce4fb9..e3661df74 100644 --- a/tests/instruments/test_xbt.py +++ b/tests/instruments/test_xbt.py @@ -38,15 +38,20 @@ def xbt_expedition(): """Minimal Expedition for Underwater_STInstrument instantiation.""" + class DummySchedule: + waypoints: ClassVar[list] = [ + Waypoint( + location=Location(1, 2), + time=BASE_TIME, + instrument=[InstrumentType.XBT], + ), + ] + + def _get_wps_in_use(self): + return self.waypoints + class DummyExpedition: - class schedule: - waypoints: ClassVar[list] = [ - Waypoint( - location=Location(1, 2), - time=BASE_TIME, - instrument=InstrumentType.XBT, - ), - ] + schedule = DummySchedule() instruments_config = InstrumentsConfig( xbt_config=XBTConfig( diff --git a/tests/make_realistic/problems/test_simulator.py b/tests/make_realistic/problems/test_simulator.py index efec12035..c3b790b0f 100644 --- a/tests/make_realistic/problems/test_simulator.py +++ b/tests/make_realistic/problems/test_simulator.py @@ -2,8 +2,6 @@ import random from datetime import datetime, timedelta -import numpy as np - from virtualship.instruments.types import InstrumentType from virtualship.make_realistic.problems.scenarios import ( GENERAL_PROBLEMS, @@ -11,14 +9,15 @@ InstrumentProblem, ) from virtualship.make_realistic.problems.simulator import ProblemSimulator -from virtualship.models.expedition import ( +from virtualship.models import ( Expedition, InstrumentsConfig, + Location, + Port, Schedule, ShipConfig, Waypoint, ) -from virtualship.models.location import Location from virtualship.utils import REPORT @@ -42,6 +41,17 @@ def _make_simple_expedition( ) waypoints.append(wp) + # bound waypoints list with Ports + waypoints.insert( + 0, Port(location=Location(-1, -1), time=sample_datetime - timedelta(hours=12)) + ) + waypoints.append( + Port( + location=Location(-1, -1), + time=sample_datetime + timedelta(days=num_waypoints + 1), + ) + ) + schedule = Schedule(waypoints=waypoints) instruments = InstrumentsConfig() ship = ShipConfig(ship_speed_knots=10.0) @@ -50,6 +60,15 @@ def _make_simple_expedition( ) +def _get_pre_departure_problem() -> GeneralProblem: + """Return a pre-departure problem class from the general problem registry.""" + problem = next(gp for gp in GENERAL_PROBLEMS if getattr(gp, "pre_departure", False)) + assert problem is not None, ( + "Need at least one pre-departure problem class in the general problem registry" + ) + return problem + + def test_select_problems_single_waypoint_returns_pre_departure(tmp_path): expedition = _make_simple_expedition(num_waypoints=1) instruments_in_expedition = expedition.get_instruments() @@ -60,7 +79,7 @@ def test_select_problems_single_waypoint_returns_pre_departure(tmp_path): assert isinstance(problems, dict) assert len(problems["problem_class"]) == 1 - assert problems["waypoint_i"] == [None] + assert problems["waypoint_i"] == [0] # port of departure is always 0th problem_cls = problems["problem_class"][0] assert isinstance(problem_cls, GeneralProblem) @@ -87,8 +106,8 @@ def test_no_instruments_no_instruments_problems(tmp_path): def test_select_problems_difficulty_level_zero(): """Selecting difficulty level 'easy' should return None (no problems selected), no matter how many waypoints.""" - for n_wps in np.arange(1, 5): # for a range of waypoint counts - expedition = _make_simple_expedition(num_waypoints=n_wps) + for n_wps in range(1, 5): # for a range of waypoint counts + expedition = _make_simple_expedition(num_waypoints=int(n_wps)) instruments_in_expedition = expedition.get_instruments() simulator = ProblemSimulator(expedition, ".") @@ -144,18 +163,33 @@ def test_hash_to_json(tmp_path): def test_has_contingency_pre_departure(tmp_path): + """Should calculate that there is not enough contingency for a pre-departure problem (with active departure port).""" expedition = _make_simple_expedition(num_waypoints=2) simulator = ProblemSimulator(expedition, str(tmp_path)) - pre_departure_problem = next( - gp for gp in GENERAL_PROBLEMS if getattr(gp, "pre_departure", False) - ) - assert pre_departure_problem is not None, ( - "Need at least one pre-departure problem class in the general problem registry" - ) + pre_departure_problem = _get_pre_departure_problem() # _has_contingency should return False for pre-departure (waypoint = None) - assert simulator._has_contingency(pre_departure_problem, None) is False + assert simulator._has_contingency(pre_departure_problem, 0) is False + + +def test_has_contingency_pre_departure_inactive_port(tmp_path): + """Should automatically return False for pre-departure problems when there is no active Port in the schedule (waypoint = None).""" + expedition = _make_simple_expedition(num_waypoints=2) + + # make port of departure inactive + expedition.schedule.waypoints[0].location = None + expedition.schedule.waypoints[0].time = None + + simulator = ProblemSimulator(expedition, str(tmp_path)) + + pre_departure_problem = _get_pre_departure_problem() + + # no active port, so no waypoint index for pre-departure problem + problem_wp_i = None + + # _has_contingency should return False for pre-departure (waypoint = None) + assert simulator._has_contingency(pre_departure_problem, problem_wp_i) is False def test_select_problems_difficulty_levels(tmp_path): @@ -239,8 +273,9 @@ def test_has_contingency_during_expedition(tmp_path): ) # short distance expedition should have contingency, long distance should not (given time between waypoints and ship speed is constant) - assert short_simulator._has_contingency(problem_cls, problem_waypoint_i=0) is True - assert long_simulator._has_contingency(problem_cls, problem_waypoint_i=0) is False + # problem_wp_i=1 corresponds to the first waypoint after departure port (waypoint 0) when departure port is active + assert short_simulator._has_contingency(problem_cls, problem_wp_i=1) is True + assert long_simulator._has_contingency(problem_cls, problem_wp_i=1) is False def test_post_expedition_report(tmp_path): @@ -273,6 +308,21 @@ def test_post_expedition_report(tmp_path): ) +def test_post_expedition_report_pre_departure_labeled_in_port(tmp_path): + """A pre-departure problem against an active departure port (wp_i=0, a Port index) should be labeled 'in-port'.""" + expedition = _make_simple_expedition(num_waypoints=2) + simulator = ProblemSimulator(expedition, str(tmp_path)) + + problems = {"problem_class": [_get_pre_departure_problem()], "waypoint_i": [0]} + + report_path = tmp_path / REPORT + simulator.post_expedition_report(problems, report_path) + + content = report_path.read_text(encoding="utf-8") + assert "Waypoint: in-port" in content + assert "Waypoint: 1" not in content + + def test_instrument_problems_only_selected_when_instruments_present(tmp_path): expedition = _make_simple_expedition(num_waypoints=3, no_instruments=True) instruments_in_expedition = expedition.get_instruments() @@ -294,9 +344,9 @@ def test_instrument_problems_only_selected_when_instruments_present(tmp_path): def test_instrument_not_present_doesnt_select_instrument_problem(tmp_path): expedition = _make_simple_expedition(num_waypoints=3, no_instruments=True) - # prescribe instruments at waypoints, for this test case each should only be present at one waypoint - expedition.schedule.waypoints[0].instrument = [InstrumentType.CTD] - expedition.schedule.waypoints[1].instrument = [ + # prescribe instruments at (non port, i.e. > 0th) waypoints, for this test case each should only be present at one waypoint + expedition.schedule.waypoints[1].instrument = [InstrumentType.CTD] + expedition.schedule.waypoints[2].instrument = [ InstrumentType.ARGO_FLOAT, InstrumentType.DRIFTER, ] diff --git a/tests/test_checkpoint.py b/tests/test_checkpoint.py index f84693c96..d6a8b9f84 100644 --- a/tests/test_checkpoint.py +++ b/tests/test_checkpoint.py @@ -5,19 +5,23 @@ import pytest from virtualship.models.checkpoint import Checkpoint -from virtualship.models.expedition import Expedition, Schedule, Waypoint +from virtualship.models.expedition import Expedition, Port, Schedule, Waypoint from virtualship.models.location import Location -from virtualship.utils import get_example_expedition +from virtualship.utils import _get_example_expedition @pytest.fixture def expedition(tmp_file): with open(tmp_file, "w") as file: - file.write(get_example_expedition()) + file.write(_get_example_expedition()) + return Expedition.from_yaml(tmp_file) -def make_dummy_checkpoint(failed_waypoint_i=None): +def make_dummy_checkpoint(problem_wp_i=None): + departure_port = Port( + location=Location(-1.0, 0.0), time=datetime(2024, 2, 1, 8, 0, 0) + ) wp1 = Waypoint( location=Location(latitude=0.0, longitude=0.0), time=datetime(2024, 2, 1, 10, 0, 0), @@ -28,9 +32,10 @@ def make_dummy_checkpoint(failed_waypoint_i=None): time=datetime(2024, 2, 1, 12, 0, 0), instrument=[], ) + arrival_port = Port(location=Location(2.0, 0.0), time=datetime(2024, 2, 1, 8, 0, 0)) - schedule = Schedule(waypoints=[wp1, wp2]) - return Checkpoint(past_schedule=schedule, failed_waypoint_i=failed_waypoint_i) + schedule = Schedule(waypoints=[departure_port, wp1, wp2, arrival_port]) + return Checkpoint(past_schedule=schedule, problem_wp_i=problem_wp_i) def test_to_and_from_yaml(tmp_path): @@ -43,26 +48,55 @@ def test_to_and_from_yaml(tmp_path): assert loaded.past_schedule.waypoints[0].time == cp.past_schedule.waypoints[0].time -def test_verify_no_failed_waypoint(expedition): - cp = make_dummy_checkpoint(failed_waypoint_i=None) +def test_verify_no_problems_encountered(expedition): + """With an empty problems dir, verify() should not raise, regardless of problem_wp_i.""" + cp = make_dummy_checkpoint(problem_wp_i=1) + expedition.schedule = cp.past_schedule cp.verify(expedition, Path("/tmp/empty")) # should not raise errors +def _write_problem_and_assert_resolution( + tmp_path, cp, expedition, problem_wp_i, delay_duration_hours, should_resolve +): + """Write an unresolved problem file, then assert whether cp.verify() resolves or rejects it.""" + problem = { + "resolved": False, + "delay_duration_hours": delay_duration_hours, + "problem_wp_i": problem_wp_i, + } + problem_file = tmp_path / "problem_1.json" + with open(problem_file, "w") as f: + json.dump(problem, f) + + if should_resolve: + cp.verify(expedition, tmp_path) + with open(problem_file) as f: + updated = json.load(f) + assert updated["resolved"] is True + else: + with pytest.raises(Exception) as excinfo: + cp.verify(expedition, tmp_path) + assert "has not been resolved in the schedule" in str(excinfo.value) + + def test_verify_past_waypoints_changed(expedition): - cp = make_dummy_checkpoint(failed_waypoint_i=1) + cp = make_dummy_checkpoint(problem_wp_i=1) + expedition.schedule = cp.past_schedule - # change past waypoints + # change a past waypoint (waypoint 1, which is within the protected prefix for problem_wp_i=1) new_wp1 = Waypoint( location=Location(latitude=0.0, longitude=0.0), time=datetime(2024, 2, 1, 11, 0, 0), instrument=None, ) - new_wp2 = Waypoint( - location=Location(latitude=1.0, longitude=1.0), - time=datetime(2024, 2, 1, 12, 0, 0), - instrument=None, + new_schedule = Schedule( + waypoints=[ + cp.past_schedule.waypoints[0], + new_wp1, + cp.past_schedule.waypoints[2], + cp.past_schedule.waypoints[-1], + ] ) - new_schedule = Schedule(waypoints=[new_wp1, new_wp2]) expedition.schedule = new_schedule with pytest.raises(Exception) as excinfo: @@ -83,6 +117,56 @@ def test_verify_problem_resolution( delay_duration_hours, should_resolve, ): + # departure port is active, so it is itself the problem waypoint (problem_wp_i=0); + # locations are kept very close together so sail time is negligible and the test + # isolates the delay-vs-buffer comparison. + departure_port = Port( + location=Location(0.0, 0.0), time=datetime(2024, 2, 1, 8, 0, 0) + ) + wp1 = Waypoint( + location=Location(latitude=0.0, longitude=0.001), + time=datetime(2024, 2, 1, 10, 0, 0), + instrument=[], + ) + arrival_port = Port( + location=Location(0.0, 0.002), time=datetime(2024, 2, 1, 12, 0, 0) + ) + past_schedule = Schedule(waypoints=[departure_port, wp1, arrival_port]) + cp = Checkpoint(past_schedule=past_schedule, problem_wp_i=0) + + # new schedule: push wp1 back by 1 hour (departure port must stay unchanged, as it + # is before the failed waypoint) + new_wp1 = Waypoint( + location=wp1.location, + time=datetime(2024, 2, 1, 11, 0, 0), + instrument=[], + ) + new_arrival_port = Port( + location=arrival_port.location, time=datetime(2024, 2, 1, 13, 0, 0) + ) + new_schedule = Schedule(waypoints=[departure_port, new_wp1, new_arrival_port]) + expedition.schedule = new_schedule + + _write_problem_and_assert_resolution( + tmp_path, cp, expedition, 0, delay_duration_hours, should_resolve + ) + + +@pytest.mark.parametrize( + "delay_duration_hours, should_resolve", + [ + (1.0, True), # pushing wp1 back by 2h absorbs a 1h delay + (5.0, False), # pushing wp1 back by 2h does not absorb a 5h delay + ], +) +def test_verify_problem_resolution_pre_departure_no_active_port( + tmp_path, + expedition, + delay_duration_hours, + should_resolve, +): + """problem_wp_i is None for a pre-departure problem with no active departure port.""" + departure_port = Port() # inactive placeholder: no location/time wp1 = Waypoint( location=Location(latitude=0.0, longitude=0.0), time=datetime(2024, 2, 1, 10, 0, 0), @@ -93,36 +177,27 @@ def test_verify_problem_resolution( time=datetime(2024, 2, 1, 12, 0, 0), instrument=[], ) - past_schedule = Schedule(waypoints=[wp1, wp2]) - cp = Checkpoint(past_schedule=past_schedule, failed_waypoint_i=1) + arrival_port = Port( + location=Location(2.0, 0.0), time=datetime(2024, 2, 1, 14, 0, 0) + ) - # new schedule - new_wp1 = wp1 + past_schedule = Schedule(waypoints=[departure_port, wp1, wp2, arrival_port]) + cp = Checkpoint(past_schedule=past_schedule, problem_wp_i=None) + + # new schedule: push wp1 (and everything after it) back by 2 hours + new_wp1 = Waypoint( + location=wp1.location, + time=datetime(2024, 2, 1, 12, 0, 0), + instrument=[], + ) new_wp2 = Waypoint( - location=Location(latitude=1.0, longitude=1.0), - time=datetime(2024, 2, 1, 20, 0, 0), + location=wp2.location, + time=datetime(2024, 2, 1, 14, 0, 0), instrument=[], ) - new_schedule = Schedule(waypoints=[new_wp1, new_wp2]) + new_schedule = Schedule(waypoints=[departure_port, new_wp1, new_wp2, arrival_port]) expedition.schedule = new_schedule - # unresolved problem file - problem = { - "resolved": False, - "delay_duration_hours": delay_duration_hours, - "problem_waypoint_i": 0, - } - problem_file = tmp_path / "problem_1.json" - with open(problem_file, "w") as f: - json.dump(problem, f) - - # check if resolution is detected correctly - if should_resolve: - cp.verify(expedition, tmp_path) - with open(problem_file) as f: - updated = json.load(f) - assert updated["resolved"] is True - else: - with pytest.raises(Exception) as excinfo: - cp.verify(expedition, tmp_path) - assert "has not been resolved in the schedule" in str(excinfo.value) + _write_problem_and_assert_resolution( + tmp_path, cp, expedition, None, delay_duration_hours, should_resolve + ) diff --git a/tests/test_mfp_to_yaml.py b/tests/test_mfp_to_yaml.py deleted file mode 100644 index 4eab16c29..000000000 --- a/tests/test_mfp_to_yaml.py +++ /dev/null @@ -1,152 +0,0 @@ -import os - -import pandas as pd -import pytest - -from virtualship.models import Expedition -from virtualship.utils import mfp_to_yaml - - -def valid_mfp_data(): - return pd.DataFrame( - { - "Station Type": ["A", "B", "C"], - "Name": ["Station1", "Station2", "Station3"], - "Latitude": [30.8, 31.2, 32.5], - "Longitude": [-44.3, -45.1, -46.7], - } - ) - - -# Fixture for Excel file -@pytest.fixture -def valid_excel_mfp_file(tmp_path): - path = tmp_path / "file.xlsx" - valid_mfp_data().to_excel(path, index=False) - return path - - -# Fixture for CSV file -@pytest.fixture -def valid_csv_mfp_file(tmp_path): - path = tmp_path / "file.csv" - valid_mfp_data().to_csv(path, index=False) - return path - - -@pytest.fixture -def valid_csv_mfp_file_with_commas(tmp_path): - path = tmp_path / "file.csv" - valid_mfp_data().to_csv(path, decimal=",", index=False) - return path - - -@pytest.fixture -def invalid_mfp_file(tmp_path): - path = tmp_path / "file.csv" - valid_mfp_data().to_csv(path, decimal=",", sep="|", index=False) - - return path - - -@pytest.fixture -def unsupported_extension_mfp_file(tmp_path): - path = tmp_path / "file.unsupported" - valid_mfp_data().to_csv(path, index=False) - - return path - - -@pytest.fixture -def nonexistent_mfp_file(tmp_path): - path = tmp_path / "non_file.csv" - - return path - - -@pytest.fixture -def missing_columns_mfp_file(tmp_path): - path = tmp_path / "file.xlsx" - valid_mfp_data().drop(columns=["Longitude"]).to_excel(path, index=False) - return path - - -@pytest.fixture -def unexpected_header_mfp_file(tmp_path): - path = tmp_path / "file.xlsx" - df = valid_mfp_data() - df["Unexpected Column"] = ["Extra1", "Extra2", "Extra3"] - df.to_excel(path, index=False) - yield path - - -@pytest.mark.parametrize( - "fixture_name", - ["valid_excel_mfp_file", "valid_csv_mfp_file", "valid_csv_mfp_file_with_commas"], -) -def test_mfp_to_yaml_success(request, fixture_name, tmp_path): - """Test that mfp_to_yaml correctly processes a valid MFP file.""" - valid_mfp_file = request.getfixturevalue(fixture_name) - - yaml_output_path = tmp_path / "expedition.yaml" - - # Run function (No need to mock open() for YAML, real file is created) - mfp_to_yaml(valid_mfp_file, yaml_output_path) - - # Ensure the YAML file was written - assert yaml_output_path.exists() - - # Load YAML and validate contents - data = Expedition.from_yaml(yaml_output_path) - - assert len(data.schedule.waypoints) == 3 - - -@pytest.mark.parametrize( - "fixture_name,error,match", - [ - pytest.param( - "nonexistent_mfp_file", - FileNotFoundError, - os.path.basename("/non_file.csv"), - id="FileNotFound", - ), - pytest.param( - "unsupported_extension_mfp_file", - RuntimeError, - "Could not read coordinates data from the provided file. Ensure it is either a csv or excel file.", - id="UnsupportedExtension", - ), - pytest.param( - "invalid_mfp_file", - ValueError, - r"Error: Found columns \['Station Type\|Name\|Latitude\|Longitude'\], but expected columns \[.*('Station Type'|'Longitude'|'Latitude'|'Name').*\]. Are you sure that you're using the correct export from MFP\?", - id="InvalidFile", - ), - pytest.param( - "missing_columns_mfp_file", - ValueError, - ( - r"Error: Found columns \[.*?('Station Type'| 'Name'| 'Latitude').*?\], " - r"but expected columns \[.*?('Station Type'| 'Name'| 'Latitude'| 'Longitude').*?\]." - ), - id="MissingColumns", - ), - ], -) -def test_mfp_to_yaml_exceptions(request, fixture_name, error, match, tmp_path): - """Test that mfp_to_yaml raises an error when input file is not valid.""" - fixture = request.getfixturevalue(fixture_name) - - yaml_output_path = tmp_path / "expedition.yaml" - - with pytest.raises(error, match=match): - mfp_to_yaml(fixture, yaml_output_path) - - -def test_mfp_to_yaml_extra_headers(unexpected_header_mfp_file, tmp_path): - """Test that mfp_to_yaml prints a warning when extra columns are found.""" - yaml_output_path = tmp_path / "expedition.yaml" - - with pytest.warns(UserWarning, match="Found additional unexpected columns.*"): - mfp_to_yaml(unexpected_header_mfp_file, yaml_output_path) diff --git a/tests/test_utils.py b/tests/test_utils.py index 63b5c4e97..53d42c29a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -10,7 +10,7 @@ import virtualship.utils from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType -from virtualship.models.expedition import Expedition, SensorConfig +from virtualship.models.expedition import Expedition, Port, SensorConfig, Waypoint from virtualship.models.location import Location from virtualship.utils import ( PROJECTION, @@ -18,17 +18,18 @@ _calc_wp_stationkeeping_time, _find_nc_file_with_variable, _get_bathy_data, + _get_example_expedition, + _get_public_wp, _select_product_id, _start_end_in_product_timerange, build_particle_class_from_sensors, - get_example_expedition, ) @pytest.fixture def expedition(tmp_file): with open(tmp_file, "w") as file: - file.write(get_example_expedition()) + file.write(_get_example_expedition()) return Expedition.from_yaml(tmp_file) @@ -68,18 +69,6 @@ def fake_open_dataset(*args, **kwargs): yield -def test_get_example_expedition(): - assert len(get_example_expedition()) > 0 - - -def test_valid_example_expedition(tmp_path): - path = tmp_path / "test.yaml" - with open(path, "w") as file: - file.write(get_example_expedition()) - - Expedition.from_yaml(path) - - def test_instrument_registry_updates(dummy_instrument): from virtualship import utils @@ -323,6 +312,23 @@ class DrifterConfig: ) +def test_get_public_wp(): + """Port waypoints have no public number; non-port waypoints are numbered 1-indexed, ignoring ports.""" + waypoints = [ + Port(location=Location(0, 0)), # index 0: departure port + Waypoint(location=Location(1, 1)), # index 1: public waypoint 1 + Port(location=Location(2, 2)), # index 2: stop-over port + Waypoint(location=Location(3, 3)), # index 3: public waypoint 2 + Port(location=Location(4, 4)), # index 4: arrival port + ] + + assert _get_public_wp(0, waypoints) is None + assert _get_public_wp(1, waypoints) == 1 + assert _get_public_wp(2, waypoints) is None + assert _get_public_wp(3, waypoints) == 2 + assert _get_public_wp(4, waypoints) is None + + def test_calc_wp_stationkeeping_time_no_instruments(expedition): """Test calc_wp_stationkeeping_time handles no instruments, either marked as 'null' or empty list.""" stationkeeping_emptylist = _calc_wp_stationkeeping_time(