diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index 21c665d01..d64a1fa2b 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -38,6 +38,100 @@ # TODO: Create evolution plots to analyze convergence +# simulate() writes one JSON object per line and reads that same shape back, so +# this is the only format it can both resume from and overwrite safely. +_SIMULATION_LOG_SUFFIX = ".txt" + + +def _refuse_logs_this_run_cannot_write( + input_file, output_file, error_file, export_config=None +): + """Reject a log file ``simulate`` would damage rather than extend. + + A ``.csv`` or ``.json`` is importable for analysis, but this run would + truncate it under ``append=False`` and leave it half one format and half + another under ``append=True``. Checked before any file is opened. + """ + for label, path in ( + ("input_file", input_file), + ("output_file", output_file), + ("error_file", error_file), + ): + if Path(path).suffix.lower() != _SIMULATION_LOG_SUFFIX: + raise ValueError( + f"Monte Carlo simulation logs must be {_SIMULATION_LOG_SUFFIX} " + f"files holding one JSON object per line; {label} is " + f"'{path}'. CSV and JSON results can be imported for analysis, " + f"but simulate() cannot resume from or overwrite them. Point " + f"{label} at a {_SIMULATION_LOG_SUFFIX} file to run." + ) + + _refuse_logs_that_are_one_file( + ( + ("input_file", input_file), + ("output_file", output_file), + ("error_file", error_file), + ) + ) + _refuse_export_options_that_break_a_line(export_config or {}) + + +def _points_at_the_same_file(one, other): + """Whether two names reach one file, by inode when both already exist. + + ``samefile`` settles symlinks, hard links and a case-insensitive filesystem, + none of which text comparison sees. It needs both to exist, so a run that has + not created them yet falls back to the resolved paths, which still normalises + ``a/../run.txt`` and any symlinked parent. + """ + one, other = Path(one), Path(other) + try: + return one.samefile(other) + except OSError: + return one.resolve() == other.resolve() + + +def _refuse_logs_that_are_one_file(labelled_paths): + """Each log has to be its own file, however the three were named. + + ``import_results`` points all three at one path, and the run then appends + input rows and output rows into it. The completeness check reports the mess + afterwards, by which time the file it was given is already gone. + """ + for index, (label, path) in enumerate(labelled_paths): + for other_label, other in labelled_paths[index + 1 :]: + if _points_at_the_same_file(path, other): + raise ValueError( + f"{label} and {other_label} are the same file ('{path}' and " + f"'{other}'). A run appends input rows and output rows " + f"separately, so sharing one log writes both into it and " + f"leaves neither readable. Give each its own file." + ) + + +def _refuse_export_options_that_break_a_line(export_config): + """Reject export options that would split one record over several lines. + + The logs hold one JSON object per line and every reader here assumes it, so + ``indent`` of any kind, ``0`` and ``""`` included, leaves a file that the + completeness check calls damaged once the run it just finished is over. + """ + if export_config.get("indent") is not None: + raise ValueError( + f"indent={export_config['indent']!r} cannot be used with a Monte " + f"Carlo run: the logs hold one JSON object per line, and an " + f"indented record spans several. Export the results with indent " + f"after the run instead." + ) + separators = export_config.get("separators") + if separators and any("\n" in str(part) for part in separators): + raise ValueError( + f"separators={separators!r} cannot be used with a Monte Carlo run: " + f"a newline inside a record splits it across lines, and the logs " + f"hold one JSON object per line." + ) + + class MonteCarlo: # pylint: disable=too-many-public-methods """Class to run a Monte Carlo simulation of a rocket flight. @@ -223,6 +317,11 @@ def simulate( self.number_of_simulations = number_of_simulations self._initial_sim_idx = self.num_of_loaded_sims if append else 0 + # Before anything is opened: __setup_files truncates for append=False. + _refuse_logs_this_run_cannot_write( + self.input_file, self.output_file, self.error_file, kwargs + ) + print("Starting Monte Carlo analysis") self.__setup_files(append) @@ -1216,7 +1315,9 @@ def import_outputs(self, filename=None): ----- Notice that you can import the outputs, inputs, and errors from a file without the need to run simulations. You can use previously saved - files to process analyze the results or to continue a simulation. + files to process and analyze the results, and a ``.txt`` one to continue + a simulation. A ``.csv`` or ``.json`` is read-only here: ``simulate`` + writes JSONL and refuses to run over a file it could not read back. """ filepath = filename if filename else self.filename.with_suffix(".outputs.txt") diff --git a/tests/unit/simulation/test_monte_carlo.py b/tests/unit/simulation/test_monte_carlo.py index 7e2e68804..bd27db221 100644 --- a/tests/unit/simulation/test_monte_carlo.py +++ b/tests/unit/simulation/test_monte_carlo.py @@ -1,5 +1,6 @@ import csv import json +import os import pathlib from collections import namedtuple @@ -8,6 +9,9 @@ import pytest from rocketpy.simulation import MonteCarlo +from rocketpy.simulation.monte_carlo import ( + _refuse_logs_this_run_cannot_write, +) plt.rcParams.update({"figure.max_open_warning": 0}) @@ -513,3 +517,149 @@ def test_simulate_convergence_runs_until_max_when_not_converging(): assert mc.num_of_loaded_sims == 200 assert all(width > 0.5 for width in history) assert len(history) == 4 # 200 / 50 batches + + +@pytest.mark.parametrize( + "suffix, payload", + [ + (".csv", "apogee,index\n1234.0,0\n1250.0,1\n"), + (".json", '[{"apogee": 1234.0, "index": 0}]\n'), + ], +) +@pytest.mark.parametrize("append", [False, True]) +def test_simulate_refuses_a_results_file_it_cannot_write( + monte_carlo_calisto, tmp_path, suffix, payload, append +): + """Importing CSV or JSON results must not let simulate() write over them. + + ``import_outputs`` accepts both and points ``output_file`` at the file, and + its docstring offers continuing a simulation. simulate() only writes JSONL, + so ``append=False`` truncated the file before this check existed. + """ + results = tmp_path / f"results{suffix}" + results.write_text(payload, encoding="utf-8") + monte_carlo_calisto.output_file = str(results) + before = results.read_bytes() + + with pytest.raises(ValueError, match="one JSON object per line"): + monte_carlo_calisto.simulate(number_of_simulations=1, append=append) + + assert results.read_bytes() == before + + +def _three_logs(tmp_path): + """Three distinct, acceptable working logs.""" + return [ + str(tmp_path / f"run.{part}.txt") for part in ("inputs", "outputs", "errors") + ] + + +def test_simulation_log_check_names_the_file_that_is_wrong(tmp_path): + """The message says which of the three paths has to change.""" + good = _three_logs(tmp_path) + + _refuse_logs_this_run_cannot_write(*good) # canonical, no raise + + for label, args in ( + ("input_file", (str(tmp_path / "a.csv"), good[1], good[2])), + ("output_file", (good[0], str(tmp_path / "b.json"), good[2])), + ("error_file", (good[0], good[1], str(tmp_path / "c.csv"))), + ): + with pytest.raises(ValueError, match=label): + _refuse_logs_this_run_cannot_write(*args) + + +def test_simulation_log_check_accepts_an_uppercase_suffix(tmp_path): + """A .TXT log is the same file to the filesystem, so it is accepted.""" + upper = [str(tmp_path / f"run.{part}.TXT") for part in ("in", "out", "err")] + _refuse_logs_this_run_cannot_write(*upper) + + +def _three_logs(tmp_path): + """Three distinct, acceptable working logs.""" + return [ + str(tmp_path / f"run.{part}.txt") for part in ("inputs", "outputs", "errors") + ] + + +def test_working_logs_must_be_three_different_files(tmp_path): + """``import_results`` points all three at one path, which cannot work. + + A run appends input rows and output rows separately, so one shared log ends + up holding both and neither reader can make sense of it. + """ + shared = str(tmp_path / "result.txt") + + with pytest.raises(ValueError, match="same file"): + _refuse_logs_this_run_cannot_write(shared, shared, shared) + + +@pytest.mark.parametrize("alias", ["dotdot", "symlink", "hardlink"]) +def test_a_log_named_two_ways_is_still_one_file(tmp_path, alias): + """Text comparison misses every way one file answers to two names.""" + inputs, _, errors = _three_logs(tmp_path) + pathlib.Path(inputs).write_text("", encoding="utf-8") + (tmp_path / "sub").mkdir() + + if alias == "dotdot": + other = str(tmp_path / "sub" / ".." / "run.inputs.txt") + else: + other = str(tmp_path / f"run.{alias}.txt") + try: + if alias == "symlink": + pathlib.Path(other).symlink_to(inputs) + else: + os.link(inputs, other) + except (OSError, NotImplementedError): + pytest.skip(f"{alias} not available on this filesystem") + + with pytest.raises(ValueError, match="same file"): + _refuse_logs_this_run_cannot_write(inputs, other, errors) + + +def test_three_separate_logs_are_accepted(tmp_path): + """The control: distinct .txt paths raise nothing.""" + _refuse_logs_this_run_cannot_write(*_three_logs(tmp_path)) + + +@pytest.mark.parametrize("indent", [2, 0, ""]) +def test_an_indented_record_is_refused_before_anything_is_written(tmp_path, indent): + """``indent`` splits a record over lines the readers take one at a time. + + Without this the run finished, then the completeness check called the file + it had just written damaged. + """ + with pytest.raises(ValueError, match="indent"): + _refuse_logs_this_run_cannot_write(*_three_logs(tmp_path), {"indent": indent}) + + +def test_a_newline_in_the_separators_is_refused_too(tmp_path): + """The same hazard by another name.""" + with pytest.raises(ValueError, match="separators"): + _refuse_logs_this_run_cannot_write( + *_three_logs(tmp_path), {"separators": (",\n", ": ")} + ) + + +@pytest.mark.parametrize( + "harmless", [{"indent": None}, {"sort_keys": True}, {"ensure_ascii": False}] +) +def test_export_options_that_keep_one_line_are_left_alone(tmp_path, harmless): + """Only what puts a newline inside a record is refused.""" + _refuse_logs_this_run_cannot_write(*_three_logs(tmp_path), harmless) + + +def test_two_names_for_a_file_that_does_not_exist_yet_are_still_one_file(tmp_path): + """``samefile`` needs both to exist, and a first run has created neither. + + Every other case here writes the file first, so the resolved-path branch + that a first run actually takes was never exercised. + """ + (tmp_path / "sub").mkdir() + missing = str(tmp_path / "run.inputs.txt") + same_by_another_name = str(tmp_path / "sub" / ".." / "run.inputs.txt") + errors = str(tmp_path / "run.errors.txt") + + assert not pathlib.Path(missing).exists() + with pytest.raises(ValueError, match="same file"): + _refuse_logs_this_run_cannot_write(missing, same_by_another_name, errors)