From 266f67729e8ad056b49bf6b9794634b6456bb276 Mon Sep 17 00:00:00 2001 From: Piotr Rozyczko Date: Fri, 24 Jul 2026 13:07:11 +0200 Subject: [PATCH] fixes for summary printout #375 --- src/easyreflectometry/fitting.py | 18 ++++++++ .../summary/html_templates.py | 2 +- src/easyreflectometry/summary/summary.py | 28 +++++------ tests/summary/test_summary.py | 46 +++++++++++++++++++ tests/test_fitting.py | 40 ++++++++++++++++ 5 files changed, 120 insertions(+), 14 deletions(-) diff --git a/src/easyreflectometry/fitting.py b/src/easyreflectometry/fitting.py index 3cb0ecb3..4cd24004 100644 --- a/src/easyreflectometry/fitting.py +++ b/src/easyreflectometry/fitting.py @@ -355,6 +355,24 @@ def fit_single_data_set_1d(self, data: DataSet1D, objective: str | None = None) ] return result + def record_fit_results(self, results: list[FitResults] | None) -> None: + """Record fit results produced outside this fitter instance. + + The application runs threaded fits directly on the lower-level + ``easy_science_multi_fitter`` (for progress reporting and cancellation) + rather than through :meth:`fit`. As a result, the high-level + ``_fit_results`` used by :attr:`chi2`, :attr:`reduced_chi`, and the + HTML summary's goodness-of-fit are never populated. Call this after such + a fit so those consumers reflect the latest results. Pass ``None`` to + clear. + + :param results: The list of ``FitResults`` from the completed fit, or + ``None`` to reset. + """ + self._fit_results = results + if not results: + self._classical_fit_metrics = None + def mcmc_sample( self, data: sc.DataGroup, diff --git a/src/easyreflectometry/summary/html_templates.py b/src/easyreflectometry/summary/html_templates.py index 9df780aa..fe520e2b 100644 --- a/src/easyreflectometry/summary/html_templates.py +++ b/src/easyreflectometry/summary/html_templates.py @@ -141,7 +141,7 @@ No. of constraints - num_constriants + num_constraints """ diff --git a/src/easyreflectometry/summary/summary.py b/src/easyreflectometry/summary/summary.py index f40f23ad..d8df7bd2 100644 --- a/src/easyreflectometry/summary/summary.py +++ b/src/easyreflectometry/summary/summary.py @@ -309,26 +309,28 @@ def _refinement_section(self) -> str: html_refinement = html_refinement.replace('num_total_params', f'{num_params}') html_refinement = html_refinement.replace('num_free_params', f'{num_free_params}') html_refinement = html_refinement.replace('num_fixed_params', f'{num_fixed_params}') - html_refinement = html_refinement.replace('num_constriants', f'{num_constraints}') + html_refinement = html_refinement.replace('num_constraints', f'{num_constraints}') return html_refinement def _compute_goodness_of_fit(self) -> str: - """Return reduced chi² as a formatted string, or 'N/A' if no fit has been run.""" - last_fit_results = getattr(self._project, '_last_fit_results', None) - if not last_fit_results: + """Return reduced chi² as a formatted string, or 'N/A' if no fit has been run. + + The value is read from the project's fitter, which computes the reduced + chi-square directly from the raw chi-square and the global degrees of + freedom across every fitted dataset. Deriving it this way keeps the + summary independent of the per-minimizer ``reduced_chi`` / ``reduced_chi2`` + attribute naming used by the underlying ``FitResults`` objects. + """ + fitter = self._project.fitter + if fitter is None: return 'N/A' try: - if len(last_fit_results) == 1: - gof = float(last_fit_results[0].reduced_chi2) - else: - total_chi2 = sum(float(r.chi2) for r in last_fit_results) - total_points = sum(len(r.x) for r in last_fit_results) - n_pars = last_fit_results[0].n_pars - dof = total_points - n_pars - gof = total_chi2 / dof if dof > 0 else 0.0 - return f'{gof:.4g}' + gof = fitter.reduced_chi except (AttributeError, TypeError, ValueError, ZeroDivisionError): return 'N/A' + if gof is None: + return 'N/A' + return f'{gof:.4g}' def _figures_section(self, interactive: bool = True) -> str: """Figures section. diff --git a/tests/summary/test_summary.py b/tests/summary/test_summary.py index 5f135fb0..271de2c4 100644 --- a/tests/summary/test_summary.py +++ b/tests/summary/test_summary.py @@ -4,6 +4,7 @@ import os from unittest.mock import MagicMock +import numpy as np import pytest from easyscience import global_object @@ -167,6 +168,51 @@ def test_refinement_section(self, project: Project) -> None: assert 'No. of free parameters:' in html assert '0' in html assert 'No. of constraints' in html + # The (previously misspelt) constraints token must be fully substituted. + assert 'num_constriants' not in html + assert 'num_constraints' not in html + + @staticmethod + def _populate_fit_results(project: Project, chi2: float, n_points: int, n_pars: int) -> None: + """Emulate a completed fit by storing results on the project's fitter.""" + fit_result = MagicMock() + fit_result.chi2 = chi2 + fit_result.x = np.arange(n_points) + fit_result.n_pars = n_pars + project.fitter._fit_results = [fit_result] + + def test_compute_goodness_of_fit_na_before_fit(self, project: Project) -> None: + # When + summary = Summary(project) + + # Then Expect: no fit has been run, so goodness-of-fit is unavailable. + assert summary._compute_goodness_of_fit() == 'N/A' + + def test_compute_goodness_of_fit_after_fit(self, project: Project) -> None: + # When: reduced chi² = 20 / (14 - 4) = 2.0 + summary = Summary(project) + self._populate_fit_results(project, chi2=20.0, n_points=14, n_pars=4) + + # Then + gof = summary._compute_goodness_of_fit() + + # Expect + assert gof != 'N/A' + assert float(gof) == pytest.approx(2.0) + + def test_refinement_section_shows_goodness_of_fit(self, project: Project) -> None: + # When + summary = Summary(project) + self._populate_fit_results(project, chi2=20.0, n_points=14, n_pars=4) + gof = summary._compute_goodness_of_fit() + + # Then + html = summary._refinement_section() + + # Expect: the template token is replaced with the actual value, not 'N/A'. + assert gof != 'N/A' + assert gof in html + assert 'goodness_of_fit' not in html def test_save_sld_plot(self, project: Project, tmp_path) -> None: # When diff --git a/tests/test_fitting.py b/tests/test_fitting.py index e4f938c8..896de593 100644 --- a/tests/test_fitting.py +++ b/tests/test_fitting.py @@ -287,6 +287,46 @@ def test_reduced_chi_uses_global_dof_across_fit_results(): assert fitter.reduced_chi == pytest.approx(expected) +def test_record_fit_results_populates_reduced_chi(): + """Results computed outside the fitter can be recorded so reduced_chi works. + + Mirrors the app path where the threaded fit runs on the low-level + easy_science_multi_fitter and the high-level results must be recorded + explicitly (otherwise the HTML summary goodness-of-fit shows 'N/A'). + """ + model = Model() + model.interface = CalculatorFactory() + fitter = MultiFitter(model) + + assert fitter.reduced_chi is None + + fit_result = MagicMock() + fit_result.chi2 = 20.0 + fit_result.x = np.arange(14) + fit_result.n_pars = 4 + + fitter.record_fit_results([fit_result]) + + assert fitter.reduced_chi == pytest.approx(20.0 / (14 - 4)) + + +def test_record_fit_results_none_clears_state(): + model = Model() + model.interface = CalculatorFactory() + fitter = MultiFitter(model) + + fit_result = MagicMock() + fit_result.chi2 = 20.0 + fit_result.x = np.arange(14) + fit_result.n_pars = 4 + fitter.record_fit_results([fit_result]) + + fitter.record_fit_results(None) + + assert fitter.reduced_chi is None + assert fitter.chi2 is None + + def test_fit_single_data_set_1d_all_zero_variance_raises(): """Legacy mask mode raises when all points have zero variance.""" model = Model()