From ce39e312c4bc00fe07aad5a360adbbfa0224f5bc Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Fri, 14 Aug 2026 13:50:52 +0200 Subject: [PATCH 1/9] large refactor of simulator logic --- .../make_realistic/problems/simulator.py | 676 ++++++++---------- 1 file changed, 315 insertions(+), 361 deletions(-) diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index e08e0827..ebeb1137 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -1,12 +1,11 @@ from __future__ import annotations import json -import os import random import sys import time from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from rich import box from rich.console import Console @@ -42,21 +41,28 @@ 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.""" @@ -67,7 +73,7 @@ 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. @@ -77,350 +83,258 @@ def select_problems( """ waypoints = self.expedition.schedule.waypoints - valid_instrument_problems = [ - problem - for problem in INSTRUMENT_PROBLEMS - if problem.instrument_type in instruments_in_expedition - ] + # handle early-exit single waypoint case (pre-departure only) + if len(waypoints) < 2: + pre_departure = [p for p in GENERAL_PROBLEMS if p.pre_departure] + return { + "problem_class": [random.choice(pre_departure)], + "waypoint_i": [None], + } - pre_departure_problems = [ + 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_problems = self._calculate_problem_count( + difficulty_level=difficulty_level, + expedition_days=(waypoints[-1].time - waypoints[0].time).days, + num_waypoints=len(waypoints), + num_instruments=len(instruments_in_expedition), + max_available=len(GENERAL_PROBLEMS) + len(valid_instruments), + ) - num_waypoints = len(waypoints) - num_instruments = len(instruments_in_expedition) - expedition_duration_days = (waypoints[-1].time - waypoints[0].time).days + if num_problems <= 0: + return None - # 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.""" if difficulty_level == "easy": - num_problems = 0 - elif difficulty_level == "medium": - num_problems = 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"]) + return 0 + if difficulty_level == "medium": + return random.randint(1, 2) + if difficulty_level == "hard": + 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) + return 0 - selected_problems = [] - problems_sorted = None - if num_problems > 0: - random.shuffle(GENERAL_PROBLEMS) - random.shuffle(valid_instrument_problems) - - # 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) - - selected_problems.extend(GENERAL_PROBLEMS[:n_general]) - selected_problems.extend(valid_instrument_problems[:n_instrument]) - - # 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 = [ + 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) + n_inst = ( + num_problems - n_gen + ) # recalc in case n_gen was capped to len(GENERAL_PROBLEMS) + + 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) + ] + + 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.expedition.schedule.waypoints + avail_indices = [ + i for i, wp in enumerate(waypoints) if not isinstance(wp, Port) + ] + random.shuffle(avail_indices) + + assigned_problems: list[ProblemType] = [] + assigned_indices: list[int | None] = [] + + for problem in selected: + if getattr(problem, "pre_departure", False): + assigned_problems.append(problem) + assigned_indices.append(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: + # 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 selected_problems - ] - available_instrument = [ - p for p in valid_instrument_problems if p not in selected_problems + if not p.pre_departure and p not in assigned_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, non-port waypoint] (or None if pre-departure) - # limited to one per waypoint, else complicates scheduling and contingency checking - waypoint_idxs = [] - unassigned_problems = [] - is_port = [isinstance(wp, Port) for wp in waypoints] - available_idxs = [i for i, port in enumerate(is_port) if not port] - - # 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: - if getattr(problem, "pre_departure", False): - waypoint_idxs.append(None) - else: - if available_idxs: - wp_select = random.choice(available_idxs) - wp_instruments = waypoints[wp_select].instrument - wp_instruments = wp_instruments if wp_instruments else [] # noqa; handle when waypoint instruments set to "null" in expedition.yaml - - # 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 - needs_replacement = ( - isinstance(problem, InstrumentProblem) - and problem.instrument_type not in wp_instruments - ) - if needs_replacement: - 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 - - waypoint_idxs.append(wp_select) - available_idxs.remove(wp_select) # each waypoint only used once - - else: - unassigned_problems.append(problem) # noqa; 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) - 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), - ) - problems_sorted = { - "problem_class": [p for p, _ in paired], - "waypoint_i": [w for _, w in paired], - } - - return problems_sorted if selected_problems else None + 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 (pre-departure/None first, then waypoint index order) + paired = sorted( + zip(assigned_problems, assigned_indices, strict=True), + key=lambda x: -1 if x[1] is None else x[1], + ) + return { + "problem_class": [p for p, _ in paired], + "waypoint_i": [w for _, w in paired], + } 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"] + continue - else: - alert_msg = LOG_MESSAGING["during_expedition"].format( - waypoint=int(problem_waypoint_i) + 1 - ) + alert_msg = ( + LOG_MESSAGING["pre_departure"] + if isinstance(problem, GeneralProblem) and problem.pre_departure + else LOG_MESSAGING["during_expedition"].format(waypoint=wp_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, + problem, wp_i, alert_msg, problem_hash, hash_fpath, log_delay ) - - # 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._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: ProblemType, problem_waypoint_i: int | None, alert_msg: str, 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.""" + # TODO: the affected waypoint messaging is wrong considering the addition of Ports + #! under the hood, waypoint_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 + + 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, - ) - + self._hash_to_json(problem, problem_hash, problem_waypoint_i, hash_fpath) has_contingency = self._has_contingency(problem, problem_waypoint_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 = self._read_json(hash_fpath) + data["resolved"] = True + self._write_json(hash_fpath, data) else: affected = ( "in-port" if problem_waypoint_i is None else f"at waypoint {problem_waypoint_i + 1}" ) - - 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" + 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" + ) 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 + ) _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,64 +342,110 @@ 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, + self, problem: ProblemType, problem_waypoint_i: int | None ) -> bool: - """Determine if enough contingency time has been scheduled to avoid delay affecting the waypoint immediately after the problem.""" + """Check whether scheduled contingency covers expected delay duration.""" if problem_waypoint_i is None: - return False # pre-departure problems always cause delay to first waypoint + return False - else: - curr_wp = self.expedition.schedule.waypoints[problem_waypoint_i] - next_wp = self.expedition.schedule.waypoints[problem_waypoint_i + 1] + waypoints = self.expedition.schedule.waypoints + curr_wp, next_wp = ( + waypoints[problem_waypoint_i], + waypoints[problem_waypoint_i + 1], + ) - wp_stationkeeping_time = _calc_wp_stationkeeping_time( - curr_wp.instrument, self.expedition - ) + stationkeeping = _calc_wp_stationkeeping_time( + curr_wp.instrument, self.expedition + ) + 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()), + } + ProblemSimulator._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 = ProblemSimulator._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} - 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") + 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) + + return {"problem_class": selected_classes, "waypoint_i": waypoint_indices} + + @staticmethod + def post_expedition_report( + 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 + ): + affected = "in-port" if wp_i is None else f"{wp_i + 1}" + 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, 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, @@ -494,58 +454,52 @@ def _hash_to_json( "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) + ProblemSimulator._write_json(hash_path, hash_data) + + @staticmethod + def _read_json(path: Path) -> dict[str, Any]: + with open(path, encoding="utf-8") as f: + return json.load(f) + + @staticmethod + def _write_json(path: Path, data: dict[str, Any]) -> None: + with open(path, "w", encoding="utf-8") as f: + json.dump(data, f, indent=4) @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) From a10c2d41cfe4c7d51b72c70073eb07696e3e8b88 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:20:32 +0200 Subject: [PATCH 2/9] start fixing public waypoint number comms, tidy up some var names --- src/virtualship/cli/_run.py | 3 +- .../expedition/simulate_schedule.py | 2 +- .../make_realistic/problems/simulator.py | 66 ++++++++++--------- src/virtualship/models/checkpoint.py | 34 +++++----- .../make_realistic/problems/test_simulator.py | 4 +- tests/test_checkpoint.py | 12 ++-- 6 files changed, 62 insertions(+), 59 deletions(-) diff --git a/src/virtualship/cli/_run.py b/src/virtualship/cli/_run.py index 6969d32f..b4a58ca9 100644 --- a/src/virtualship/cli/_run.py +++ b/src/virtualship/cli/_run.py @@ -97,6 +97,7 @@ def _run( checkpoint = Checkpoint(past_schedule=Schedule(waypoints=[])) # verify that schedule and checkpoint match, and that problems have been resolved + # TODO: problems related verifcation should probably be refactored out of the Checkpoint class (which is Pydantic model) and into the ProblemSimulator class (which is more appropriate for handling problems) checkpoint.verify(expedition, problems_dir) print("\n---- WAYPOINT VERIFICATION ----") @@ -121,7 +122,7 @@ def _run( _save_checkpoint( Checkpoint( past_schedule=expedition.schedule, - failed_waypoint_i=schedule_results.failed_waypoint_i, + failed_wp=schedule_results.failed_wp, ), expedition_dir, ) diff --git a/src/virtualship/expedition/simulate_schedule.py b/src/virtualship/expedition/simulate_schedule.py index 6f1fed05..516c1491 100644 --- a/src/virtualship/expedition/simulate_schedule.py +++ b/src/virtualship/expedition/simulate_schedule.py @@ -36,7 +36,7 @@ class ScheduleProblem: """Result of schedule that could not be fully completed.""" time: datetime - failed_waypoint_i: int + failed_wp: int @dataclass diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index ebeb1137..e41ffb96 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -237,7 +237,7 @@ def _assign_problems_to_waypoints( if not assigned_problems: return None - # Sort chronologically (pre-departure/None first, then waypoint index order) + # sort chronologically (pre-departure/None first, then waypoint index order) paired = sorted( zip(assigned_problems, assigned_indices, strict=True), key=lambda x: -1 if x[1] is None else x[1], @@ -269,37 +269,43 @@ def execute( if hash_fpath.exists(): continue - alert_msg = ( - LOG_MESSAGING["pre_departure"] - if isinstance(problem, GeneralProblem) and problem.pre_departure - else LOG_MESSAGING["during_expedition"].format(waypoint=wp_i + 1) - ) - - self._log_problem( - problem, wp_i, alert_msg, problem_hash, hash_fpath, log_delay - ) + self._log_problem(problem, wp_i, problem_hash, hash_fpath, log_delay) self._cache_original_expedition(self.expedition) def _log_problem( self, problem: ProblemType, - problem_waypoint_i: int | None, - alert_msg: str, + problem_wp_i: int | None, problem_hash: str, hash_fpath: Path, log_delay: float, ) -> None: - """Handle execution sequence, logging, checkpoint saving, and user presentation.""" - # TODO: the affected waypoint messaging is wrong considering the addition of Ports - #! under the hood, waypoint_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 + """ + 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.expedition.schedule.waypoints + non_port_wps = [i for i, wp in enumerate(waypoints) if not isinstance(wp, Port)] + public_wp = ( + non_port_wps.index(problem_wp_i) + 1 if problem_wp_i is not None else None + ) + + 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: @@ -310,11 +316,8 @@ def _log_problem( data["resolved"] = True self._write_json(hash_fpath, data) else: - affected = ( - "in-port" - if problem_waypoint_i is None - else f"at waypoint {problem_waypoint_i + 1}" - ) + breakpoint() + affected = "in-port" if problem_wp_i 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" @@ -328,8 +331,9 @@ def _log_problem( # 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 + failed_wp=problem_wp_i + + 1 # TODO: should this use user_facing_wp_i instead of problem_wp_i? + if problem_wp_i is not None else 0, ) _save_checkpoint(checkpoint, self.expedition_dir) @@ -345,17 +349,15 @@ def _log_problem( if not has_contingency: sys.exit(0) - def _has_contingency( - self, problem: ProblemType, problem_waypoint_i: int | None - ) -> bool: + def _has_contingency(self, problem: ProblemType, problem_wp_i: int | None) -> bool: """Check whether scheduled contingency covers expected delay duration.""" - if problem_waypoint_i is None: + if problem_wp_i is None: return False waypoints = self.expedition.schedule.waypoints curr_wp, next_wp = ( - waypoints[problem_waypoint_i], - waypoints[problem_waypoint_i + 1], + waypoints[problem_wp_i], + waypoints[problem_wp_i + 1], ) stationkeeping = _calc_wp_stationkeeping_time( @@ -442,14 +444,14 @@ def post_expedition_report( def _hash_to_json( problem: ProblemType, problem_hash: str, - problem_waypoint_i: int | None, + problem_wp_i: int | None, hash_path: Path, ) -> 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, diff --git a/src/virtualship/models/checkpoint.py b/src/virtualship/models/checkpoint.py index ce620af1..a304c676 100644 --- a/src/virtualship/models/checkpoint.py +++ b/src/virtualship/models/checkpoint.py @@ -37,7 +37,7 @@ class Checkpoint(pydantic.BaseModel): """ past_schedule: Schedule - failed_waypoint_i: int | None = None + failed_wp: int | None = None def to_yaml(self, file_path: str | Path) -> None: """ @@ -69,14 +69,14 @@ 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: + if self.failed_wp is None: pass elif ( - not new_schedule.waypoints[: int(self.failed_waypoint_i)] - == self.past_schedule.waypoints[: int(self.failed_waypoint_i)] + not new_schedule.waypoints[: int(self.failed_wp)] + == self.past_schedule.waypoints[: int(self.failed_wp)] ): 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 {int(self.failed_wp) + 1} onwards)." ) # 2) check that problems have been resolved in the new schedule @@ -98,12 +98,12 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: problem_waypoint = ( new_schedule.waypoints[0] - if problem["problem_waypoint_i"] is None - else new_schedule.waypoints[problem["problem_waypoint_i"]] + if problem["problem_wp_i"] is None + else new_schedule.waypoints[problem["problem_wp_i"]] ) # 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: + if problem["problem_wp_i"] is None: time_diff = ( problem_waypoint.time - self.past_schedule.waypoints[0].time ) @@ -111,7 +111,7 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: # 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] + failed_waypoint = new_schedule.waypoints[self.failed_wp] scheduled_time = failed_waypoint.time - problem_waypoint.time @@ -149,27 +149,27 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: else: problem_wp_str = ( "in-port" - if problem["problem_waypoint_i"] is None - else f"at waypoint {problem['problem_waypoint_i'] + 1}" + if problem["problem_wp_i"] is None + else f"at waypoint {problem['problem_wp_i'] + 1}" ) affected_wp_str = ( "1" - if problem["problem_waypoint_i"] is None - else f"{problem['problem_waypoint_i'] + 2}" + if problem["problem_wp_i"] is None + else f"{problem['problem_wp_i'] + 2}" ) time_elapsed = ( (sail_time + delay_duration + stationkeeping_time) - if problem["problem_waypoint_i"] is not None + if problem["problem_wp_i"] is not None else delay_duration ) failed_waypoint_time = ( failed_waypoint.time - if problem["problem_waypoint_i"] is not None + if problem["problem_wp_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 + if problem["problem_wp_i"] is not None else self.past_schedule.waypoints[0].time + time_elapsed ) @@ -179,7 +179,7 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: 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 + if problem["problem_wp_i"] is not None else "" ) ) diff --git a/tests/make_realistic/problems/test_simulator.py b/tests/make_realistic/problems/test_simulator.py index ccd1de18..f1241b37 100644 --- a/tests/make_realistic/problems/test_simulator.py +++ b/tests/make_realistic/problems/test_simulator.py @@ -235,8 +235,8 @@ 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 + assert short_simulator._has_contingency(problem_cls, problem_wp_i=0) is True + assert long_simulator._has_contingency(problem_cls, problem_wp_i=0) is False def test_post_expedition_report(tmp_path): diff --git a/tests/test_checkpoint.py b/tests/test_checkpoint.py index f84693c9..b6cb60f1 100644 --- a/tests/test_checkpoint.py +++ b/tests/test_checkpoint.py @@ -17,7 +17,7 @@ def expedition(tmp_file): return Expedition.from_yaml(tmp_file) -def make_dummy_checkpoint(failed_waypoint_i=None): +def make_dummy_checkpoint(failed_wp=None): wp1 = Waypoint( location=Location(latitude=0.0, longitude=0.0), time=datetime(2024, 2, 1, 10, 0, 0), @@ -30,7 +30,7 @@ def make_dummy_checkpoint(failed_waypoint_i=None): ) schedule = Schedule(waypoints=[wp1, wp2]) - return Checkpoint(past_schedule=schedule, failed_waypoint_i=failed_waypoint_i) + return Checkpoint(past_schedule=schedule, failed_wp=failed_wp) def test_to_and_from_yaml(tmp_path): @@ -44,12 +44,12 @@ def test_to_and_from_yaml(tmp_path): def test_verify_no_failed_waypoint(expedition): - cp = make_dummy_checkpoint(failed_waypoint_i=None) + cp = make_dummy_checkpoint(failed_wp=None) cp.verify(expedition, Path("/tmp/empty")) # should not raise errors def test_verify_past_waypoints_changed(expedition): - cp = make_dummy_checkpoint(failed_waypoint_i=1) + cp = make_dummy_checkpoint(failed_wp=1) # change past waypoints new_wp1 = Waypoint( @@ -94,7 +94,7 @@ def test_verify_problem_resolution( instrument=[], ) past_schedule = Schedule(waypoints=[wp1, wp2]) - cp = Checkpoint(past_schedule=past_schedule, failed_waypoint_i=1) + cp = Checkpoint(past_schedule=past_schedule, failed_wp=1) # new schedule new_wp1 = wp1 @@ -110,7 +110,7 @@ def test_verify_problem_resolution( problem = { "resolved": False, "delay_duration_hours": delay_duration_hours, - "problem_waypoint_i": 0, + "problem_wp_i": 0, } problem_file = tmp_path / "problem_1.json" with open(problem_file, "w") as f: From d4ccffeccf1fd10acae0cf339e0d52582088423a Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:48:11 +0200 Subject: [PATCH 3/9] refactor getting public facing wp number to utils method --- src/virtualship/instruments/base.py | 4 ++-- src/virtualship/make_realistic/problems/simulator.py | 11 +++-------- src/virtualship/utils.py | 9 ++++++++- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index e06ae344..bdeeaa2e 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -26,9 +26,9 @@ _find_files_in_timerange, _find_nc_file_with_variable, _get_bathy_data, + _get_clean_encoding, _get_waypoint_latlons, _select_product_id, - get_clean_encoding, ship_spinner, ) @@ -319,7 +319,7 @@ def _get_local_ds(self, files: list[Path]) -> xr.Dataset: @staticmethod def _via_tmp_ds(ds: xr.Dataset) -> xr.Dataset: """Create and re-load a temporary local dataset.""" - encoding = get_clean_encoding(ds) + encoding = _get_clean_encoding(ds) with tempfile.TemporaryDirectory() as tmpdir: tmp_fpath = Path(tmpdir) / "tmp.nc" diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index e41ffb96..ca98ba88 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -31,6 +31,7 @@ PROJECTION, _calc_sail_time, _calc_wp_stationkeeping_time, + _get_public_wp, _make_hash, _save_checkpoint, ) @@ -288,10 +289,7 @@ def _log_problem( problem_wp_i will often == public_wp (given 0-indexing), but this makes the logic explicit and clear. """ waypoints = self.expedition.schedule.waypoints - non_port_wps = [i for i, wp in enumerate(waypoints) if not isinstance(wp, Port)] - public_wp = ( - non_port_wps.index(problem_wp_i) + 1 if problem_wp_i is not None else None - ) + public_wp = _get_public_wp(problem_wp_i, waypoints) alert_msg = ( LOG_MESSAGING["pre_departure"] @@ -331,10 +329,7 @@ def _log_problem( # update and save checkpoints checkpoint = Checkpoint( past_schedule=self.expedition.schedule, - failed_wp=problem_wp_i - + 1 # TODO: should this use user_facing_wp_i instead of problem_wp_i? - if problem_wp_i is not None - else 0, + failed_wp_i=problem_wp_i if problem_wp_i is not None else 0, ) _save_checkpoint(checkpoint, self.expedition_dir) self.expedition.to_yaml(self.expedition_dir / CACHE / EXPEDITION_LATEST) diff --git a/src/virtualship/utils.py b/src/virtualship/utils.py index 9441defd..5da00c9e 100644 --- a/src/virtualship/utils.py +++ b/src/virtualship/utils.py @@ -15,6 +15,7 @@ from parcels import FieldSet, Particle, Variable from virtualship.errors import CopernicusCatalogueError +from virtualship.models.expedition import Port if TYPE_CHECKING: from virtualship.expedition.simulate_schedule import ( @@ -525,7 +526,7 @@ def build_particle_class_from_sensors( return Particle.add_variable(nonsensor_variables + sensor_variables) -def get_clean_encoding(ds): +def _get_clean_encoding(ds): """ Clean existing encodings and supply explicit native endianness to prevent netCDF4 UserWarnings. @@ -539,6 +540,12 @@ def get_clean_encoding(ds): return encoding +def _get_public_wp(raw_wp_i: int | None, waypoints: list) -> int | None: + """Get the public waypoint index for a given waypoint (accounting for Port waypoints).""" + non_port_wps = [i for i, wp in enumerate(waypoints) if not isinstance(wp, Port)] + return non_port_wps.index(raw_wp_i) + 1 if raw_wp_i is not None else None + + # ===================================================== # SECTION: misc. # ===================================================== From 77c69a7ea223d41e96f5a900ea115e2e09ad5b17 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:51:17 +0200 Subject: [PATCH 4/9] next steps of adapting public facing wp numbers --- src/virtualship/models/checkpoint.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/virtualship/models/checkpoint.py b/src/virtualship/models/checkpoint.py index a304c676..84b02093 100644 --- a/src/virtualship/models/checkpoint.py +++ b/src/virtualship/models/checkpoint.py @@ -17,6 +17,7 @@ PROJECTION, _calc_sail_time, _calc_wp_stationkeeping_time, + _get_public_wp, ) @@ -37,7 +38,7 @@ class Checkpoint(pydantic.BaseModel): """ past_schedule: Schedule - failed_wp: int | None = None + failed_wp_i: int | None = None def to_yaml(self, file_path: str | Path) -> None: """ @@ -68,15 +69,20 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: """ new_schedule = expedition.schedule + # get the public waypoint number of the failed waypoint (if any), for use in error messages + public_failed_wp = _get_public_wp( + self.failed_wp_i + 1, self.past_schedule.waypoints + ) + # 1) check that past waypoints have not been changed, unless is a pre-departure problem - if self.failed_wp is None: + if self.failed_wp_i is None: pass elif ( - not new_schedule.waypoints[: int(self.failed_wp)] - == self.past_schedule.waypoints[: int(self.failed_wp)] + not new_schedule.waypoints[: int(self.failed_wp_i + 1)] + == self.past_schedule.waypoints[: int(self.failed_wp_i + 1)] ): raise CheckpointError( - f"Past waypoints in schedule have been changed! Restore past schedule and only change future waypoints (waypoint {int(self.failed_wp) + 1} onwards)." + f"Past waypoints in schedule have been changed! Restore past schedule and only change future waypoints (waypoint {int(public_failed_wp) + 1} onwards)." # +1 because it's the waypoint after the failed waypoint ) # 2) check that problems have been resolved in the new schedule @@ -103,6 +109,8 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: ) # pre-departure problem: check that whole delay duration has been added to first waypoint time (by testing against past schedule) + # TODO: just taking the 0th waypoint doesn't work anymore given expedition has Port information now! + #! TODO: could combine into one single check that applies to all waypoints now that Ports have locations, rather than hypothetical? if problem["problem_wp_i"] is None: time_diff = ( problem_waypoint.time - self.past_schedule.waypoints[0].time @@ -111,7 +119,7 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: # 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_wp] + failed_waypoint = new_schedule.waypoints[self.failed_wp_i + 1] scheduled_time = failed_waypoint.time - problem_waypoint.time From b62438234049fdcd2de9aa50b73aadec55b30ca1 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:14:45 +0200 Subject: [PATCH 5/9] continue refactor work + using waypoint index for in-port problems --- src/virtualship/cli/_run.py | 10 +- .../make_realistic/problems/simulator.py | 58 +++++---- src/virtualship/models/checkpoint.py | 118 +++++++----------- src/virtualship/utils.py | 20 ++- 4 files changed, 96 insertions(+), 110 deletions(-) diff --git a/src/virtualship/cli/_run.py b/src/virtualship/cli/_run.py index b4a58ca9..5d0614a5 100644 --- a/src/virtualship/cli/_run.py +++ b/src/virtualship/cli/_run.py @@ -14,7 +14,7 @@ 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, @@ -93,12 +93,10 @@ 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 - # TODO: problems related verifcation should probably be refactored out of the Checkpoint class (which is Pydantic model) and into the ProblemSimulator class (which is more appropriate for handling problems) - 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 ----") diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index ca98ba88..92b98605 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -4,6 +4,7 @@ import random import sys import time +from datetime import timedelta from pathlib import Path from typing import TYPE_CHECKING, Any @@ -70,6 +71,14 @@ def __init__(self, expedition: Expedition, expedition_dir: str | Path): self.expedition = expedition self.expedition_dir = Path(expedition_dir) + self.waypoints = expedition.schedule.waypoints + + def __post_init__(self): + """Ensure first and last waypoints are Ports. Allows the problem selection to work properly.""" + assert isinstance(self.waypoints[0], Port) & isinstance( + self.waypoints[-1], Port + ), "First and last waypoints must be Port types." + def select_problems( self, instruments_in_expedition: set[InstrumentType], @@ -80,16 +89,14 @@ def select_problems( 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). Finally, cache the suite of problems to a directory (expedition-specific) for reference. """ - waypoints = self.expedition.schedule.waypoints - # handle early-exit single waypoint case (pre-departure only) - if len(waypoints) < 2: + if len(self.waypoints) < 2: pre_departure = [p for p in GENERAL_PROBLEMS if p.pre_departure] return { "problem_class": [random.choice(pre_departure)], - "waypoint_i": [None], + "waypoint_i": [0], # noqa; pre-departure problem is always associated with the departure port (index 0) } valid_instruments = [ @@ -99,8 +106,8 @@ def select_problems( ] num_problems = self._calculate_problem_count( difficulty_level=difficulty_level, - expedition_days=(waypoints[-1].time - waypoints[0].time).days, - num_waypoints=len(waypoints), + expedition_days=(self.waypoints[-1].time - self.waypoints[0].time).days, + num_waypoints=len(self.waypoints), num_instruments=len(instruments_in_expedition), max_available=len(GENERAL_PROBLEMS) + len(valid_instruments), ) @@ -152,9 +159,7 @@ def _sample_problems( 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) - n_inst = ( - num_problems - n_gen - ) # recalc in case n_gen was capped to len(GENERAL_PROBLEMS) + n_inst = num_problems - n_gen # noqa; recalc in case n_gen was capped to len(GENERAL_PROBLEMS) return general_pool[:n_gen] + instrument_pool[:n_inst] @@ -189,19 +194,23 @@ def _assign_problems_to_waypoints( self, selected: list[ProblemType] ) -> SelectedProblemsDict | None: """Assign sampled problems to valid, non-port waypoint indices.""" - waypoints = self.expedition.schedule.waypoints + waypoints = self.waypoints avail_indices = [ i for i, wp in enumerate(waypoints) if not isinstance(wp, Port) ] random.shuffle(avail_indices) + assert 0 not in avail_indices, ( + "Index 0 (departure port) should not be in available waypoint indices for non-pre-departure problems." + ) + assigned_problems: list[ProblemType] = [] assigned_indices: list[int | None] = [] for problem in selected: if getattr(problem, "pre_departure", False): assigned_problems.append(problem) - assigned_indices.append(None) + assigned_indices.append(0) # noqa; pre-departure problem is always associated with the departure port (index 0) continue if not avail_indices: @@ -238,10 +247,10 @@ def _assign_problems_to_waypoints( if not assigned_problems: return None - # sort chronologically (pre-departure/None first, then waypoint index order) + # sort chronologically (waypoint 0 first, then remaining waypoint index order) paired = sorted( zip(assigned_problems, assigned_indices, strict=True), - key=lambda x: -1 if x[1] is None else x[1], + key=lambda x: 0 if x[1] == 0 else x[1], ) return { "problem_class": [p for p, _ in paired], @@ -288,7 +297,7 @@ def _log_problem( 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.expedition.schedule.waypoints + waypoints = self.waypoints public_wp = _get_public_wp(problem_wp_i, waypoints) alert_msg = ( @@ -314,8 +323,7 @@ def _log_problem( data["resolved"] = True self._write_json(hash_fpath, data) else: - breakpoint() - affected = "in-port" if problem_wp_i is None else f"at waypoint {public_wp}" + 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" @@ -329,7 +337,7 @@ def _log_problem( # update and save checkpoints checkpoint = Checkpoint( past_schedule=self.expedition.schedule, - failed_wp_i=problem_wp_i if problem_wp_i is not None else 0, + problem_wp_i=problem_wp_i, ) _save_checkpoint(checkpoint, self.expedition_dir) self.expedition.to_yaml(self.expedition_dir / CACHE / EXPEDITION_LATEST) @@ -346,17 +354,15 @@ def _log_problem( def _has_contingency(self, problem: ProblemType, problem_wp_i: int | None) -> bool: """Check whether scheduled contingency covers expected delay duration.""" - if problem_wp_i is None: - return False - - waypoints = self.expedition.schedule.waypoints curr_wp, next_wp = ( - waypoints[problem_wp_i], - waypoints[problem_wp_i + 1], + self.waypoints[problem_wp_i], + self.waypoints[problem_wp_i + 1], ) - stationkeeping = _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, diff --git a/src/virtualship/models/checkpoint.py b/src/virtualship/models/checkpoint.py index 84b02093..a0631922 100644 --- a/src/virtualship/models/checkpoint.py +++ b/src/virtualship/models/checkpoint.py @@ -11,7 +11,7 @@ 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, @@ -38,7 +38,7 @@ class Checkpoint(pydantic.BaseModel): """ past_schedule: Schedule - failed_wp_i: int | None = None + problem_wp_i: int | None = None def to_yaml(self, file_path: str | Path) -> None: """ @@ -69,20 +69,25 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: """ new_schedule = expedition.schedule - # get the public waypoint number of the failed waypoint (if any), for use in error messages - public_failed_wp = _get_public_wp( - self.failed_wp_i + 1, self.past_schedule.waypoints + # TODO: problems related verifcation should probably be refactored out of the Checkpoint class (which is Pydantic model) and into the ProblemSimulator class (which is more appropriate for handling problems) + #! Do some re-thinking to move all the problems related logic over into the Problems (simulator). + + # failed waypoint is the waypoint immediately *after* the problem waypoint (i.e. the one that will not be reached in time) + failed_wp_i = self.problem_wp_i + 1 + + # public waypoint number of problem and failed waypoints, for use in error messages + public_problem_wp = _get_public_wp( + self.problem_wp_i, self.past_schedule.waypoints ) + public_failed_wp = _get_public_wp(failed_wp_i, self.past_schedule.waypoints) - # 1) check that past waypoints have not been changed, unless is a pre-departure problem - if self.failed_wp_i is None: - pass - elif ( - not new_schedule.waypoints[: int(self.failed_wp_i + 1)] - == self.past_schedule.waypoints[: int(self.failed_wp_i + 1)] + # 1) check that past waypoints have not been changed (up to but not including failed_wp) + if ( + not new_schedule.waypoints[: int(failed_wp_i)] + == self.past_schedule.waypoints[: int(failed_wp_i)] ): raise CheckpointError( - f"Past waypoints in schedule have been changed! Restore past schedule and only change future waypoints (waypoint {int(public_failed_wp) + 1} onwards)." # +1 because it's the waypoint after the failed waypoint + f"Past waypoints in schedule have been changed! Restore past schedule and only change future waypoints (waypoint {int(public_failed_wp)} onwards)." ) # 2) check that problems have been resolved in the new schedule @@ -94,54 +99,38 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: for file in hash_fpaths: with open(file, encoding="utf-8") as f: problem = json.load(f) + + # continue if problem is already resolved, else perform checks to see if delay is accounted for 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) + else: delay_duration = timedelta( hours=float(problem["delay_duration_hours"]) ) - problem_waypoint = ( - new_schedule.waypoints[0] - if problem["problem_wp_i"] is None - else new_schedule.waypoints[problem["problem_wp_i"]] - ) - - # pre-departure problem: check that whole delay duration has been added to first waypoint time (by testing against past schedule) - # TODO: just taking the 0th waypoint doesn't work anymore given expedition has Port information now! - #! TODO: could combine into one single check that applies to all waypoints now that Ports have locations, rather than hypothetical? - if problem["problem_wp_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_wp_i + 1] - - scheduled_time = failed_waypoint.time - problem_waypoint.time + problem_waypoint = new_schedule.waypoints[self.problem_wp_i] + failed_waypoint = new_schedule.waypoints[failed_wp_i] + scheduled_time_diff = failed_waypoint.time - problem_waypoint.time - stationkeeping_time = _calc_wp_stationkeeping_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 ) + 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] - resolved = scheduled_time >= min_time_required + min_time_required = sail_time + delay_duration + stationkeeping_time - if resolved: + if scheduled_time_diff >= min_time_required: print( "\n\nšŸŽ‰ Previous problem has been resolved in the schedule.\n" ) @@ -157,37 +146,18 @@ def verify(self, expedition: Expedition, problems_dir: Path) -> None: else: problem_wp_str = ( "in-port" - if problem["problem_wp_i"] is None - else f"at waypoint {problem['problem_wp_i'] + 1}" - ) - affected_wp_str = ( - "1" - if problem["problem_wp_i"] is None - else f"{problem['problem_wp_i'] + 2}" - ) - time_elapsed = ( - (sail_time + delay_duration + stationkeeping_time) - if problem["problem_wp_i"] is not None - else delay_duration - ) - failed_waypoint_time = ( - failed_waypoint.time - if problem["problem_wp_i"] is not None - else new_schedule.waypoints[0].time - ) - current_time = ( - problem_waypoint.time + time_elapsed - if problem["problem_wp_i"] is not None - else self.past_schedule.waypoints[0].time + time_elapsed + if problem["problem_wp_i"] == 0 # i.e. pre-departure + else f"at waypoint {public_problem_wp}" ) + time_elapsed = sail_time + delay_duration + stationkeeping_time 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"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 {problem_waypoint.time + time_elapsed}, 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_wp_i"] is not None + 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 problem["problem_wp_i"] != 0 else "" ) ) diff --git a/src/virtualship/utils.py b/src/virtualship/utils.py index 5da00c9e..58431b3e 100644 --- a/src/virtualship/utils.py +++ b/src/virtualship/utils.py @@ -15,7 +15,6 @@ from parcels import FieldSet, Particle, Variable from virtualship.errors import CopernicusCatalogueError -from virtualship.models.expedition import Port if TYPE_CHECKING: from virtualship.expedition.simulate_schedule import ( @@ -541,9 +540,22 @@ def _get_clean_encoding(ds): def _get_public_wp(raw_wp_i: int | None, waypoints: list) -> int | None: - """Get the public waypoint index for a given waypoint (accounting for Port waypoints).""" - non_port_wps = [i for i, wp in enumerate(waypoints) if not isinstance(wp, Port)] - return non_port_wps.index(raw_wp_i) + 1 if raw_wp_i is not None else 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 # ===================================================== From 73e5204ed2e61b76c9c5d7124a53163f2ae42ac1 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:51:23 +0200 Subject: [PATCH 6/9] refactor: separate problem-specific checkpoint verification logic from core checkpoint model, move away from reliance on problem-specific tracking via json tmp files --- src/virtualship/cli/_run.py | 23 ++- .../make_realistic/problems/simulator.py | 80 +++++++- src/virtualship/models/checkpoint.py | 176 +++++------------- 3 files changed, 128 insertions(+), 151 deletions(-) diff --git a/src/virtualship/cli/_run.py b/src/virtualship/cli/_run.py index 5d0614a5..348111ca 100644 --- a/src/virtualship/cli/_run.py +++ b/src/virtualship/cli/_run.py @@ -42,11 +42,7 @@ def _run( expedition_dir: str | Path, difficulty_level: str, from_data: Path | None = None ) -> None: - """ - Perform an expedition, providing terminal feedback and file output. - - :param expedition_dir: The base directory for the expedition. - """ + """Perform an expedition, providing terminal feedback and file output.""" # start timing start_time = time.time() print("[TIMER] Expedition started...") @@ -91,12 +87,18 @@ def _run( # verify instruments_config file is consistent with schedule expedition.instruments_config.verify(expedition) - # load last checkpoint + # initialise problem simulator + problem_simulator = ProblemSimulator(expedition, expedition_dir) + + # load last checkpoint if present checkpoint = _load_checkpoint(expedition_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) + # 1) core structural check: verify past waypoints have not changed + checkpoint.verify_past_schedule(expedition.schedule) + + # 2) problems-specific check: verify active problem delay is resolved in new schedule + problem_simulator.verify_problem_resolution(checkpoint) print("\n---- WAYPOINT VERIFICATION ----") @@ -120,7 +122,7 @@ def _run( _save_checkpoint( Checkpoint( past_schedule=expedition.schedule, - failed_wp=schedule_results.failed_wp, + failed_wp_i=schedule_results.failed_wp, ), expedition_dir, ) @@ -144,9 +146,6 @@ def _run( # identify instruments in expedition instruments_in_expedition = expedition.get_instruments() - # initialise problem simulator - problem_simulator = ProblemSimulator(expedition, expedition_dir) - # re-load previously encountered (same expedition as previously) problems if they exist, else select new problems and cache them if os.path.exists(problems_dir.joinpath(SELECTED_PROBLEMS)): problems = problem_simulator.load_selected_problems( diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index 92b98605..1ae2ba33 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -15,6 +15,7 @@ from rich.table import Table from yaspin import yaspin +from virtualship.errors import CheckpointError from virtualship.instruments.types import InstrumentType from virtualship.make_realistic.problems.scenarios import ( GENERAL_PROBLEMS, @@ -22,7 +23,7 @@ GeneralProblem, InstrumentProblem, ) -from virtualship.models.checkpoint import Checkpoint +from virtualship.models.checkpoint import ActiveProblem, Checkpoint from virtualship.models.expedition import Port from virtualship.utils import ( CACHE, @@ -282,6 +283,61 @@ def execute( self._log_problem(problem, wp_i, problem_hash, hash_fpath, log_delay) self._cache_original_expedition(self.expedition) + def verify_problem_resolution(self, checkpoint: Checkpoint) -> None: + """Verify active problem delay is resolved in new schedule.""" + active_problem = checkpoint.active_problem + if active_problem is None or active_problem.resolved: + return + + failed_wp_i = checkpoint.get_effective_failed_wp_i() + new_schedule = self.expedition.schedule + + # problem-specific delay calculation & resolution check + delay_duration = timedelta(hours=active_problem.delay_duration_hours) + problem_waypoint = new_schedule.waypoints[checkpoint.problem_wp_i] + failed_waypoint = new_schedule.waypoints[failed_wp_i] + + scheduled_time_diff = failed_waypoint.time - problem_waypoint.time + stationkeeping_time = ( + _calc_wp_stationkeeping_time(problem_waypoint.instrument, self.expedition) + if not isinstance(problem_waypoint, Port) + else timedelta(0) + ) + sail_time = _calc_sail_time( + problem_waypoint.location, + failed_waypoint.location, + ship_speed_knots=self.expedition.ship_config.ship_speed_knots, + projection=PROJECTION, + )[0] + + min_time_required = sail_time + delay_duration + stationkeeping_time + + if scheduled_time_diff >= min_time_required: + print("\n\nšŸŽ‰ Previous problem has been resolved in the schedule.\n") + active_problem.resolved = True + _save_checkpoint(checkpoint, self.expedition_dir) + else: + public_problem_wp = _get_public_wp( + checkpoint.problem_wp_i, checkpoint.past_schedule.waypoints + ) + public_failed_wp = _get_public_wp( + failed_wp_i, checkpoint.past_schedule.waypoints + ) + problem_wp_str = ( + "in-port" + if checkpoint.problem_wp_i == 0 + else f"at waypoint {public_problem_wp}" + ) + time_elapsed = sail_time + delay_duration + stationkeeping_time + + raise CheckpointError( + f"The problem encountered in previous simulation has not been resolved in the schedule! " + f"Please adjust the schedule to account for delays caused by the problem...\n\n" + f"The problem was associated with a delay duration of {active_problem.delay_duration_hours} hours {problem_wp_str} " + f"(meaning waypoint {public_failed_wp} could not be reached in time). " + f"Currently, the ship would reach waypoint {public_failed_wp} at {problem_waypoint.time + time_elapsed}, but the scheduled time is {failed_waypoint.time}." + ) + def _log_problem( self, problem: ProblemType, @@ -293,9 +349,10 @@ def _log_problem( """ 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. + 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 should use public_wp. + Incidentally, problem_wp_i will often == public_wp (given 0-indexing), but this makes the logic explicit and clear. """ waypoints = self.waypoints public_wp = _get_public_wp(problem_wp_i, waypoints) @@ -311,17 +368,13 @@ def _log_problem( time.sleep(log_delay) spinner.ok("šŸ’„ ") - 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 state to resolved - data = self._read_json(hash_fpath) - data["resolved"] = True - self._write_json(hash_fpath, data) + active_problem = None else: affected = "in-port" if public_wp is None else f"at waypoint {public_wp}" impact_str = ( @@ -333,11 +386,18 @@ def _log_problem( problem_wp=affected, expedition_yaml=EXPEDITION, ) + active_problem = ActiveProblem( + message=problem.message, + problem_wp_i=problem_wp_i, + delay_duration_hours=delay_hrs, + resolved=False, + ) - # update and save checkpoints + # update and save checkpoint with active problem information checkpoint = Checkpoint( past_schedule=self.expedition.schedule, problem_wp_i=problem_wp_i, + active_problem=active_problem, ) _save_checkpoint(checkpoint, self.expedition_dir) self.expedition.to_yaml(self.expedition_dir / CACHE / EXPEDITION_LATEST) diff --git a/src/virtualship/models/checkpoint.py b/src/virtualship/models/checkpoint.py index a0631922..8ce43326 100644 --- a/src/virtualship/models/checkpoint.py +++ b/src/virtualship/models/checkpoint.py @@ -2,8 +2,6 @@ from __future__ import annotations -import json -from datetime import timedelta from pathlib import Path import pydantic @@ -11,14 +9,8 @@ from virtualship.errors import CheckpointError from virtualship.instruments.types import InstrumentType -from virtualship.models.expedition import Expedition, Port, Schedule -from virtualship.utils import ( - EXPEDITION, - PROJECTION, - _calc_sail_time, - _calc_wp_stationkeeping_time, - _get_public_wp, -) +from virtualship.models.expedition import Schedule +from virtualship.utils import _get_public_wp class _YamlDumper(yaml.SafeDumper): @@ -30,134 +22,60 @@ class _YamlDumper(yaml.SafeDumper): ) -class Checkpoint(pydantic.BaseModel): - """ - A checkpoint of schedule simulation. - - Copy of the schedule until where the simulation proceeded without troubles. - """ - - past_schedule: Schedule - problem_wp_i: int | None = None - - def to_yaml(self, file_path: str | Path) -> None: - """ - Write checkpoint to yaml file. - - :param file_path: Path to the file to write to. - """ - with open(file_path, "w") as file: - yaml.dump(self.model_dump(by_alias=True), file, Dumper=_YamlDumper) - - @classmethod - def from_yaml(cls, file_path: str | Path) -> Checkpoint: - """ - Load checkpoint from yaml file. - - :param file_path: Path to the file to load from. - :returns: The checkpoint. - """ - with open(file_path) as file: - data = yaml.safe_load(file) - return Checkpoint(**data) +class ActiveProblem(pydantic.BaseModel): + """Runtime state of a problem halting simulation.""" - def verify(self, expedition: Expedition, problems_dir: Path) -> None: - """ - Verify that the given schedule matches the checkpoint's past schedule , and/or that any problem has been resolved. + message: str + problem_wp_i: int | None + delay_duration_hours: float + resolved: bool = False - Addresses changes made by the user in response to both i) scheduling issues arising for not enough time for the ship to travel between waypoints, and ii) problems encountered during simulation. - """ - new_schedule = expedition.schedule - # TODO: problems related verifcation should probably be refactored out of the Checkpoint class (which is Pydantic model) and into the ProblemSimulator class (which is more appropriate for handling problems) - #! Do some re-thinking to move all the problems related logic over into the Problems (simulator). +class Checkpoint(pydantic.BaseModel): + """A checkpoint of the schedule simulation storing past schedule state and any active problem that halted execution.""" - # failed waypoint is the waypoint immediately *after* the problem waypoint (i.e. the one that will not be reached in time) - failed_wp_i = self.problem_wp_i + 1 + past_schedule: Schedule + problem_wp_i: int | None = ( + None # index of the waypoint that caused a problem (if any) + ) + failed_wp_i: int | None = ( + None # index of the waypoint that could not be reached in time (either because of problem or incompatible user scheduling) + ) + active_problem: ActiveProblem | None = None + + def get_effective_failed_wp_i(self) -> int | None: + """Return the index of the waypoint that failed or could not be reached.""" + if self.failed_wp_i is not None: + return self.failed_wp_i + if self.problem_wp_i is not None: + return self.problem_wp_i + 1 + return None + + def verify_past_schedule(self, new_schedule: Schedule) -> None: + """Core structural check: ensure past history hasn't been edited.""" + failed_wp_i = self.get_effective_failed_wp_i() + if failed_wp_i is None: + return - # public waypoint number of problem and failed waypoints, for use in error messages - public_problem_wp = _get_public_wp( - self.problem_wp_i, self.past_schedule.waypoints - ) 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[: int(failed_wp_i)] - == self.past_schedule.waypoints[: int(failed_wp_i)] + new_schedule.waypoints[: int(failed_wp_i)] + != self.past_schedule.waypoints[: int(failed_wp_i)] ): raise CheckpointError( - f"Past waypoints in schedule have been changed! Restore past schedule and only change future waypoints (waypoint {int(public_failed_wp)} onwards)." + f"Past waypoints in schedule have been changed! Restore past schedule " + f"and only change future waypoints (waypoint {int(public_failed_wp)} onwards)." ) - # 2) check that problems have been resolved in the new schedule - 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) - - # continue if problem is already resolved, else perform checks to see if delay is accounted for - if problem["resolved"]: - continue - else: - delay_duration = timedelta( - hours=float(problem["delay_duration_hours"]) - ) - - problem_waypoint = new_schedule.waypoints[self.problem_wp_i] - failed_waypoint = new_schedule.waypoints[failed_wp_i] - scheduled_time_diff = failed_waypoint.time - problem_waypoint.time - - 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] - - min_time_required = sail_time + delay_duration + stationkeeping_time - - if scheduled_time_diff >= 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 - 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_wp_i"] == 0 # i.e. pre-departure - else f"at waypoint {public_problem_wp}" - ) - time_elapsed = sail_time + delay_duration + stationkeeping_time - - 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 {problem_waypoint.time + time_elapsed}, 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 problem["problem_wp_i"] != 0 - else "" - ) - ) + def to_yaml(self, file_path: str | Path) -> None: + """Write checkpoint to YAML file.""" + with open(file_path, "w", encoding="utf-8") as file: + yaml.dump(self.model_dump(by_alias=True), file, Dumper=_YamlDumper) + + @classmethod + def from_yaml(cls, file_path: str | Path) -> Checkpoint: + """Load checkpoint from YAML file.""" + with open(file_path, encoding="utf-8") as file: + data = yaml.safe_load(file) + return Checkpoint(**data) From 24dc97c49ade69b8127a060a79bc18f5d7458665 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:27:31 +0200 Subject: [PATCH 7/9] user messaging fix when failed wp is port of arrival --- src/virtualship/make_realistic/problems/simulator.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index 1ae2ba33..32cf1b4c 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -323,6 +323,9 @@ def verify_problem_resolution(self, checkpoint: Checkpoint) -> None: public_failed_wp = _get_public_wp( failed_wp_i, checkpoint.past_schedule.waypoints ) + if public_failed_wp is None: + public_failed_wp = "\b/Port of Arrival" + problem_wp_str = ( "in-port" if checkpoint.problem_wp_i == 0 From b7398458926a7f123525e2499ab6719ed3b45ef4 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:59:34 +0200 Subject: [PATCH 8/9] fix check for unique expedition --- src/virtualship/cli/_run.py | 43 +++++++----- .../make_realistic/problems/simulator.py | 69 ++++++++++++------- 2 files changed, 71 insertions(+), 41 deletions(-) diff --git a/src/virtualship/cli/_run.py b/src/virtualship/cli/_run.py index 348111ca..b4401822 100644 --- a/src/virtualship/cli/_run.py +++ b/src/virtualship/cli/_run.py @@ -75,9 +75,9 @@ def _run( expedition = _get_expedition(expedition_dir) - # 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) + # unique id to determine if an expedition has 'changed' since last run cache_dir = expedition_dir.joinpath(CACHE) - expedition_id = _unique_id(expedition, cache_dir) + expedition_id = _unique_id(expedition, cache_dir, expedition_dir) # dedicated problems directory for this expedition problems_dir = expedition_dir.joinpath( @@ -128,13 +128,15 @@ def _run( ) return - # delete and create results directory + # warn about existing results on fresh runs (when no active checkpoint exists) results_dir = expedition_dir.joinpath(RESULTS) - _warn_overwrite_results_dir(results_dir) + if checkpoint is None: + _warn_overwrite_results_dir(results_dir) - if os.path.exists(results_dir): + # re-initialize/clean results directory + if os.path.exists(results_dir) and checkpoint is None: shutil.rmtree(results_dir) - os.makedirs(results_dir) + os.makedirs(results_dir, exist_ok=True) print("\n----- EXPEDITION SUMMARY ------") @@ -146,7 +148,7 @@ def _run( # identify instruments in expedition instruments_in_expedition = expedition.get_instruments() - # re-load previously encountered (same expedition as previously) problems if they exist, else select new problems and cache them + # re-load previously encountered problems if they exist, else select new problems and cache them if os.path.exists(problems_dir.joinpath(SELECTED_PROBLEMS)): problems = problem_simulator.load_selected_problems( problems_dir.joinpath(SELECTED_PROBLEMS) @@ -155,20 +157,16 @@ def _run( problems = problem_simulator.select_problems( instruments_in_expedition, difficulty_level ) - problem_simulator.cache_selected_problems( - problems, problems_dir.joinpath(SELECTED_PROBLEMS) - ) if problems else None + if problems: + problem_simulator.cache_selected_problems( + problems, problems_dir.joinpath(SELECTED_PROBLEMS) + ) # simulate instrument measurements print("\nSimulating measurements. This may take a while...\n") for itype in instruments_in_expedition: try: - # get instrument class - instrument_class = get_instrument_class(itype) - if instrument_class is None: - raise RuntimeError(f"No instrument class found for type {itype}.") - # execute problem simulations for this instrument type if problems: if ( @@ -185,6 +183,11 @@ def _run( log_dir=problems_dir, ) + # get instrument class + instrument_class = get_instrument_class(itype) + if instrument_class is None: + raise RuntimeError(f"No instrument class found for type {itype}.") + # get measurements to simulate attr = MeasurementsToSimulate.get_attr_for_instrumenttype(itype) measurements = getattr(schedule_results.measurements_to_simulate, attr) @@ -247,7 +250,7 @@ def _run( print(f"[TIMER] Expedition completed in {elapsed / 60.0:.2f} minutes.") -def _unique_id(expedition: Expedition, cache_dir: Path) -> str: +def _unique_id(expedition: Expedition, cache_dir: Path, expedition_dir: Path) -> str: """ Return a unique id for the expedition (marked by datetime), which can be used to determine whether the expedition has 'changed' since the last run. @@ -258,6 +261,7 @@ def _unique_id(expedition: Expedition, cache_dir: Path) -> str: id_path = cache_dir.joinpath(EXPEDITION_IDENTIFIER) last_expedition_path = cache_dir.joinpath(EXPEDITION_LATEST) + checkpoint_path = expedition_dir.joinpath(CHECKPOINT) new_id = datetime.now().strftime("%Y%m%d%H%M%S") if not id_path.exists(): @@ -266,10 +270,13 @@ def _unique_id(expedition: Expedition, cache_dir: Path) -> str: previous_id = id_path.read_text().strip() + # if an active checkpoint exists, retain the existing expedition id to preserve problem state + if checkpoint_path.exists(): + return previous_id + try: last_expedition = Expedition.from_yaml(last_expedition_path) except FileNotFoundError: - # cache is not useful in this case as it implies the previous run was interrupted and is incomplete; update passively id_path.write_text(new_id) return new_id @@ -277,7 +284,7 @@ def _unique_id(expedition: Expedition, cache_dir: Path) -> str: last_expedition.get_instruments() ) if not added_instruments: - return previous_id # if no additions, keep previous id to allow re-use of previously encountered problems + return previous_id id_path.write_text(new_id) return new_id diff --git a/src/virtualship/make_realistic/problems/simulator.py b/src/virtualship/make_realistic/problems/simulator.py index 32cf1b4c..8234af62 100644 --- a/src/virtualship/make_realistic/problems/simulator.py +++ b/src/virtualship/make_realistic/problems/simulator.py @@ -28,9 +28,12 @@ from virtualship.utils import ( CACHE, EXPEDITION, + EXPEDITION_IDENTIFIER, EXPEDITION_LATEST, EXPEDITION_ORIGINAL, + PROBLEMS_ENCOUNTERED, PROJECTION, + SELECTED_PROBLEMS, _calc_sail_time, _calc_wp_stationkeeping_time, _get_public_wp, @@ -61,7 +64,7 @@ } ProblemType = GeneralProblem | InstrumentProblem -SelectedProblemsDict = dict[str, list[ProblemType | None]] +SelectedProblemsDict = dict[str, Any] class ProblemSimulator: @@ -71,9 +74,16 @@ 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 + @property + def expedition_id(self) -> str: + """Retrieve the current expedition unique identifier from cache.""" + id_path = self.expedition_dir.joinpath(CACHE, EXPEDITION_IDENTIFIER) + if id_path.exists(): + return id_path.read_text().strip() + return "" + def __post_init__(self): """Ensure first and last waypoints are Ports. Allows the problem selection to work properly.""" assert isinstance(self.waypoints[0], Port) & isinstance( @@ -256,6 +266,7 @@ def _assign_problems_to_waypoints( return { "problem_class": [p for p, _ in paired], "waypoint_i": [w for _, w in paired], + "resolved": False, } def execute( @@ -266,9 +277,15 @@ def execute( log_delay: float = 4.0, ) -> None: """Execute simulation problems and apply delay/schedule impacts.""" + if not problems or problems.get("resolved", False): + return + for problem, wp_i in zip( problems["problem_class"], problems["waypoint_i"], strict=True ): + if getattr(problem, "resolved", False): + continue + if ( isinstance(problem, InstrumentProblem) and problem.instrument_type is not instrument_type_validation @@ -316,6 +333,22 @@ def verify_problem_resolution(self, checkpoint: Checkpoint) -> None: print("\n\nšŸŽ‰ Previous problem has been resolved in the schedule.\n") active_problem.resolved = True _save_checkpoint(checkpoint, self.expedition_dir) + + # persist resolved status to selected_problems.json cache + problems_path = self.expedition_dir.joinpath( + CACHE, + PROBLEMS_ENCOUNTERED.format(expedition_id=self.expedition_id), + SELECTED_PROBLEMS, + ) + if problems_path.exists(): + problems = self.load_selected_problems(problems_path) + if isinstance(problems, dict): + problems["resolved"] = True + for p in problems.get("problem_class", []): + if getattr(p, "message", None) == active_problem.message: + p.resolved = True + self.cache_selected_problems(problems, problems_path) + else: public_problem_wp = _get_public_wp( checkpoint.problem_wp_i, checkpoint.past_schedule.waypoints @@ -399,7 +432,6 @@ def _log_problem( # update and save checkpoint with active problem information checkpoint = Checkpoint( past_schedule=self.expedition.schedule, - problem_wp_i=problem_wp_i, active_problem=active_problem, ) _save_checkpoint(checkpoint, self.expedition_dir) @@ -457,6 +489,7 @@ def cache_selected_problems( payload = { "problem_class": [p.short_name for p in problems["problem_class"]], "waypoint_i": problems["waypoint_i"], + "resolved": problems.get("resolved", False), "timestamp": time.strftime("%Y-%m-%d %H:%M:%S", time.localtime()), } ProblemSimulator._write_json(fpath, payload) @@ -485,7 +518,11 @@ def load_selected_problems( ) waypoint_indices.append(wp_idx) - return {"problem_class": selected_classes, "waypoint_i": waypoint_indices} + return { + "problem_class": selected_classes, + "waypoint_i": waypoint_indices, + "resolved": data.get("resolved", False), + } @staticmethod def post_expedition_report( @@ -504,24 +541,6 @@ def post_expedition_report( f"Delay caused: {delay_hrs} hours\n\n" ) - @staticmethod - def _hash_to_json( - problem: ProblemType, - problem_hash: str, - problem_wp_i: int | None, - hash_path: Path, - ) -> None: - """Serialize runtime problem detail to JSON.""" - hash_data = { - "problem_hash": problem_hash, - "message": problem.message, - "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, - } - ProblemSimulator._write_json(hash_path, hash_data) - @staticmethod def _read_json(path: Path) -> dict[str, Any]: with open(path, encoding="utf-8") as f: @@ -536,7 +555,11 @@ def _write_json(path: Path, data: dict[str, Any]) -> None: 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.""" + """ + 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 From 69877ab772c36dc8118103e8c8d2bc857e122e6b Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:59:49 +0200 Subject: [PATCH 9/9] remove duplicate problem_wp_i declaration --- src/virtualship/models/checkpoint.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/virtualship/models/checkpoint.py b/src/virtualship/models/checkpoint.py index 8ce43326..3b939fc1 100644 --- a/src/virtualship/models/checkpoint.py +++ b/src/virtualship/models/checkpoint.py @@ -26,7 +26,7 @@ class ActiveProblem(pydantic.BaseModel): """Runtime state of a problem halting simulation.""" message: str - problem_wp_i: int | None + problem_wp_i: int | None # noqa; index of the waypoint that caused a problem (if any) delay_duration_hours: float resolved: bool = False @@ -35,14 +35,14 @@ class Checkpoint(pydantic.BaseModel): """A checkpoint of the schedule simulation storing past schedule state and any active problem that halted execution.""" past_schedule: Schedule - problem_wp_i: int | None = ( - None # index of the waypoint that caused a problem (if any) - ) - failed_wp_i: int | None = ( - None # index of the waypoint that could not be reached in time (either because of problem or incompatible user scheduling) - ) + failed_wp_i: int | None = None # noqa; index of the waypoint that could not be reached in time active_problem: ActiveProblem | None = None + @property + def problem_wp_i(self) -> int | None: + """Delegate to active_problem to avoid duplication.""" + return self.active_problem.problem_wp_i if self.active_problem else None + def get_effective_failed_wp_i(self) -> int | None: """Return the index of the waypoint that failed or could not be reached.""" if self.failed_wp_i is not None: