Bank to det renaming (#82)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: jmborr

AGENTS.md

@@ -141,15 +141,17 @@ The main reduction model is in `src/usansred/reduce.py`.
   `USANS_<run>_monitor_scan_ARN.txt` and
   `USANS_<run>_detector_scan_ARN_peak_<bank>.txt`.
 - `CombinedSample` combines raw scan data from multiple samples before reduction.
-- `XYData`, `IQData`, and `MonitorData` live in `src/usansred/model.py`.
+- `XYData`, `IQData`, and `MonitorData` live in `src/usansred/models/data.py`.
 
 Output names currently include:
 
 - `UN_<sample>_det_1_unscaled.txt`
 - `UN_<sample>_det_1.txt`
-- `UN_<sample>_det_1_lb.txt`
+- `UN_<sample>_det_<n>_unscaled.txt` and `UN_<sample>_det_<n>.txt` for harmonics
+  `n = 2..4`, only when `save_all_harmonics` is enabled. These are written flat in
+  the output directory, not in per-bank subdirectories.
 - `UN_<sample>_det_1_background_subtracted.txt` (only when a background or
-  empty cell was subtracted)
+  empty cell was subtracted; first harmonic only)
 - `summary.xlsx`
 
 ## Coding Standards

docs/source/api/usansred.rst

@@ -24,10 +24,18 @@ usansred.io.save
    :undoc-members:
    :show-inheritance:
 
-usansred.model
---------------
+usansred.models.data
+--------------------
+
+.. automodule:: usansred.models.data
+   :members:
+   :undoc-members:
+   :show-inheritance:
+
+usansred.models.reduction\_config
+---------------------------------
 
-.. automodule:: usansred.model
+.. automodule:: usansred.models.reduction_config
    :members:
    :undoc-members:
    :show-inheritance:

docs/source/user/reduce.rst

@@ -247,3 +247,18 @@ Once reduction is finished, subdirectory ``result/`` is created containing the f
   + ``UN_X5D2_8_det_1_background_subtracted.txt`` (**_background_subtracted.txt**)
     is the data after background (or empty-cell) subtraction.
     It is only written when a background or empty cell was actually subtracted.
+
+The ``_det_1`` infix denotes the first harmonic (first detector bank), the only one written
+by default. When ``save_all_harmonics`` is enabled in the JSON setup file, the unscaled and
+scaled files are written for every harmonic, using the same names with the harmonic number
+in place of the ``1``:
+
+.. code-block:: text
+
+   UN_X5D2_8_det_1_unscaled.txt   UN_X5D2_8_det_1.txt
+   UN_X5D2_8_det_2_unscaled.txt   UN_X5D2_8_det_2.txt
+   UN_X5D2_8_det_3_unscaled.txt   UN_X5D2_8_det_3.txt
+   UN_X5D2_8_det_4_unscaled.txt   UN_X5D2_8_det_4.txt
+
+Background subtraction is currently applied to the first harmonic only, so
+``_det_1_background_subtracted.txt`` has no counterpart for the higher harmonics.

src/usansred/reduce.py

@@ -347,49 +347,72 @@ def dump_reduced_data_to_csv(
         """Write this measurement's reduced data to CSV text files in the experiment's output directory.
 
         Each flag enables one category of output file (all default to True; the reduction
-        workflow in ``Experiment.dump_reduced_data`` always uses the defaults). A category
-        is also skipped when its corresponding data is empty.
+        workflow in ``Experiment.dump_reduced_data`` always uses the defaults). Higher
+        harmonics with no data are skipped with a warning, but the first harmonic is
+        required for the ``detector_data`` and ``scaled_data`` categories.
 
         Parameters
         ----------
         detector_data : bool
             Write the stitched, monitor-normalized data, ``UN_<name>_det_1_unscaled.txt``.
-            With ``save_all_harmonics``, higher banks go to ``bank_<n>/UN_<name>_unscaled.txt``.
-            Skipped when no detector data is present.
+            With ``save_all_harmonics``, higher harmonics go to ``UN_<name>_det_<n>_unscaled.txt``.
         scaled_data : bool
             Write the data rescaled by analyzer solid angle, sample thickness, and transmission,
-            ``UN_<name>_det_1.txt``. With ``save_all_harmonics``, higher banks go to
-            ``bank_<n>/UN_<name>.txt``.
+            ``UN_<name>_det_1.txt``. With ``save_all_harmonics``, higher harmonics go to
+            ``UN_<name>_det_<n>.txt``.
         background_subtracted_data : bool
             Write the background- (or empty-cell-) subtracted data,
             ``UN_<name>_det_1_background_subtracted.txt``. Only written when a subtraction
             actually occurred (``is_reduced`` is True).
+
+        Raises
+        ------
+        RuntimeError
+            If ``detector_data`` or ``scaled_data`` is requested but the first harmonic of
+            the corresponding data is missing or empty.
         """
-        if detector_data and self.data:
-            filepath = os.path.join(self.experiment.output_dir, f"UN_{self.name}_det_1_unscaled.txt")
-            self.dump_data_to_csv(filepath, self.data)
-            if self.config.save_all_harmonics:
-                for i in range(1, self.num_of_banks):
-                    bank = i + 1  # start with the second order
-                    filepath = os.path.join(
-                        self.experiment.output_dir,
-                        f"bank_{bank}",
-                        f"UN_{self.name}_unscaled.txt",
-                    )
-                    self.dump_data_to_csv(filepath, self.detector_data[i])
+        # Harmonic (detector bank) n is written to ``_det_<n>``; only the first harmonic is
+        # written unless ``save_all_harmonics`` is set.
+        num_of_harmonics = self.num_of_banks if self.config.save_all_harmonics else 1
+
+        def has_data(data: IQData) -> bool:
+            return any((data.q, data.i, data.e, data.t))
+
+        if detector_data and (not self.detector_data or not has_data(self.detector_data[0])):
+            raise RuntimeError(f"Cannot write detector data for {self.label}: first harmonic data is missing.")
+
+        if scaled_data and (not self.data_scaled or not has_data(self.data_scaled[0])):
+            raise RuntimeError(f"Cannot write scaled data for {self.label}: first harmonic data is missing.")
+
+        if detector_data:
+            missing_harmonics = []
+            for harmonic in range(1, min(num_of_harmonics, len(self.detector_data)) + 1):
+                if not has_data(self.detector_data[harmonic - 1]):
+                    missing_harmonics.append(harmonic)
+                    continue
+                filepath = os.path.join(self.experiment.output_dir, f"UN_{self.name}_det_{harmonic}_unscaled.txt")
+                self.dump_data_to_csv(filepath, self.detector_data[harmonic - 1])
+            missing_harmonics.extend(range(len(self.detector_data) + 1, num_of_harmonics + 1))
+            if missing_harmonics:
+                logger.warning(
+                    f"No detector data is available for {self.label} for harmonics {missing_harmonics}; "
+                    "skipping those data dumps."
+                )
 
         if scaled_data:
-            filepath = os.path.join(self.experiment.output_dir, f"UN_{self.name}_det_1.txt")
-            self.dump_data_to_csv(filepath, self.data_scaled[0])
-            if self.config.save_all_harmonics:
-                for i in range(1, self.num_of_banks):
-                    bank = i + 1  # start with the second order
-                    filepath = os.path.join(
-                        self.experiment.output_dir,
-                        f"bank_{bank}",
-                        f"UN_{self.name}.txt",
-                    )
-                    self.dump_data_to_csv(filepath, self.data_scaled[i])
+            missing_harmonics = []
+            for harmonic in range(1, min(num_of_harmonics, len(self.data_scaled)) + 1):
+                if not has_data(self.data_scaled[harmonic - 1]):
+                    missing_harmonics.append(harmonic)
+                    continue
+                filepath = os.path.join(self.experiment.output_dir, f"UN_{self.name}_det_{harmonic}.txt")
+                self.dump_data_to_csv(filepath, self.data_scaled[harmonic - 1])
+            missing_harmonics.extend(range(len(self.data_scaled) + 1, num_of_harmonics + 1))
+            if missing_harmonics:
+                logger.warning(
+                    f"No scaled data is available for {self.label} for harmonics {missing_harmonics}; "
+                    "skipping those data dumps."
+                )
 
         if background_subtracted_data:
             # Only written when a background or empty cell was actually subtracted

tests/integration/test_reduce.py

@@ -152,19 +152,21 @@ def test_main_save_all_harmonics(mock_parse_args, data_server, tmp_path):
 
     reduce()
 
-    for bank in range(2, 5):
-        bank_dir = output_dir / f"bank_{bank}"
-        assert bank_dir.is_dir()
+    # Higher harmonics are written flat in the output directory as ``_det_<n>``, alongside
+    # the first-harmonic ``_det_1`` files; no per-bank subdirectories are created.
+    for harmonic in range(2, 5):
         for name in ["EmptyPCell", "S115_dry", "S115_pc3"]:
-            harmonic_file = bank_dir / f"UN_{name}_unscaled.txt"
+            harmonic_file = output_dir / f"UN_{name}_det_{harmonic}_unscaled.txt"
             assert harmonic_file.is_file()
             assert harmonic_file.stat().st_size > 0
             assert len(harmonic_file.read_text(encoding="utf-8").splitlines()) == 65
 
-            scaled_file = bank_dir / f"UN_{name}.txt"
+            scaled_file = output_dir / f"UN_{name}_det_{harmonic}.txt"
             assert scaled_file.is_file()
             assert scaled_file.stat().st_size > 0
 
+    assert not list(output_dir.glob("bank_*")), "per-bank subdirectories should no longer be created"
+
     assert_reduction_log_files(
         output_dir, [("S115_pc3", "sample"), ("S115_dry", "sample"), ("EmptyPCell", "background")]
     )

tests/unit/test_sample.py

@@ -455,6 +455,98 @@ def test_dump_uneven_lists(self, mock_experiment):
             os.unlink(filepath)
 
 
+class TestDumpAllHarmonics:
+    """Tests for the ``save_all_harmonics`` output files of Sample.dump_reduced_data_to_csv."""
+
+    @staticmethod
+    def _make_two_bank_sample(experiment: Experiment, save_all_harmonics: bool, tmp_path) -> Sample:
+        experiment.output_dir = str(tmp_path)
+        experiment._config.save_all_harmonics = save_all_harmonics
+        sample = _make_sample(experiment, "test", [])
+        # One (Q,I,E) curve per detector bank, both for the unscaled and the rescaled data
+        sample.detector_data = [
+            IQData(q=[0.1, 0.2], i=[100.0, 200.0], e=[10.0, 14.0]),
+            IQData(q=[0.3, 0.4], i=[300.0, 400.0], e=[17.0, 20.0]),
+        ]
+        sample.data_scaled = [
+            IQData(q=[0.1, 0.2], i=[1.0, 2.0], e=[0.1, 0.2]),
+            IQData(q=[0.3, 0.4], i=[3.0, 4.0], e=[0.3, 0.4]),
+        ]
+        return sample
+
+    def test_writes_one_file_per_harmonic(self, mock_experiment_2banks, tmp_path):
+        """With save_all_harmonics, each bank is written flat as ``_det_<n>``."""
+        sample = self._make_two_bank_sample(mock_experiment_2banks, True, tmp_path)
+
+        sample.dump_reduced_data_to_csv(background_subtracted_data=False)
+
+        for harmonic in (1, 2):
+            assert (tmp_path / f"UN_test_det_{harmonic}_unscaled.txt").is_file()
+            assert (tmp_path / f"UN_test_det_{harmonic}.txt").is_file()
+        assert not list(tmp_path.glob("bank_*")), "no per-bank subdirectories should be created"
+
+    def test_higher_harmonics_hold_their_own_bank_data(self, mock_experiment_2banks, tmp_path):
+        """The ``_det_2`` files hold bank 2's curve, not a copy of bank 1's."""
+        sample = self._make_two_bank_sample(mock_experiment_2banks, True, tmp_path)
+
+        sample.dump_reduced_data_to_csv(background_subtracted_data=False)
+
+        rows = list(csv.reader((tmp_path / "UN_test_det_2_unscaled.txt").read_text().splitlines()))
+        assert [float(row[0]) for row in rows] == [0.3, 0.4]
+        assert [float(row[1]) for row in rows] == [300.0, 400.0]
+
+    def test_only_first_harmonic_without_the_flag(self, mock_experiment_2banks, tmp_path):
+        """Without save_all_harmonics, only the first harmonic is written."""
+        sample = self._make_two_bank_sample(mock_experiment_2banks, False, tmp_path)
+
+        sample.dump_reduced_data_to_csv(background_subtracted_data=False)
+
+        assert (tmp_path / "UN_test_det_1_unscaled.txt").is_file()
+        assert (tmp_path / "UN_test_det_1.txt").is_file()
+        assert not (tmp_path / "UN_test_det_2_unscaled.txt").exists()
+        assert not (tmp_path / "UN_test_det_2.txt").exists()
+
+    def test_missing_first_detector_harmonic_aborts_output(self, mock_experiment_2banks, tmp_path):
+        """Missing first-harmonic detector data must abort output generation."""
+        sample = self._make_two_bank_sample(mock_experiment_2banks, True, tmp_path)
+        sample.detector_data = []
+
+        with pytest.raises(RuntimeError, match="first harmonic data is missing"):
+            sample.dump_reduced_data_to_csv(scaled_data=False, background_subtracted_data=False)
+
+    def test_missing_first_scaled_harmonic_aborts_output(self, mock_experiment_2banks, tmp_path):
+        """Missing first-harmonic scaled data must abort output generation."""
+        sample = self._make_two_bank_sample(mock_experiment_2banks, True, tmp_path)
+        sample.data_scaled = []
+
+        with pytest.raises(RuntimeError, match="first harmonic data is missing"):
+            sample.dump_reduced_data_to_csv(detector_data=False, background_subtracted_data=False)
+
+    def test_missing_higher_harmonics_warns_and_skips_files(self, mock_experiment_2banks, tmp_path, caplog):
+        """Missing higher harmonics should be warned about and skipped."""
+        sample = self._make_two_bank_sample(mock_experiment_2banks, True, tmp_path)
+        sample.detector_data = sample.detector_data[:1]
+        sample.data_scaled = sample.data_scaled[:1]
+
+        with caplog.at_level(logging.WARNING):
+            sample.dump_reduced_data_to_csv(background_subtracted_data=False)
+
+        assert (tmp_path / "UN_test_det_1_unscaled.txt").is_file()
+        assert (tmp_path / "UN_test_det_1.txt").is_file()
+        assert not (tmp_path / "UN_test_det_2_unscaled.txt").exists()
+        assert not (tmp_path / "UN_test_det_2.txt").exists()
+        assert "No detector data is available" in caplog.text
+        assert "No scaled data is available" in caplog.text
+
+    def test_empty_first_scaled_harmonic_aborts_output(self, mock_experiment_2banks, tmp_path):
+        """An empty first-harmonic scaled curve must abort output generation."""
+        sample = self._make_two_bank_sample(mock_experiment_2banks, True, tmp_path)
+        sample.data_scaled[0] = IQData()
+
+        with pytest.raises(RuntimeError, match="first harmonic data is missing"):
+            sample.dump_reduced_data_to_csv(detector_data=False, background_subtracted_data=False)
+
+
 class TestDumpBackgroundSubtracted:
     """Tests for dumping the background-subtracted data file."""