BUG: refuse to run a Monte Carlo over a results file it cannot write

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

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)