fixing summary using only 1st model #435

src/easyreflectometry/summary/summary.py

@@ -102,23 +102,37 @@
 
 
 def _named_parameters(project: Project):
-    """``(display name, parameter)`` for every parameter of every model, each once:
-    the model's name, the name of the object holding the parameter, the parameter's."""
-    seen: set[int] = set()
+    """``(display name, parameter)`` for every parameter of every model, each once.
 
-    def walk(obj, model_name: str, owner_name: str):
+    The display name is the name of the model, of the object holding the parameter,
+    and the parameter's own. A parameter shared by several models (a contrast reusing
+    an assembly of its reference) is one value fitted for all of them, so it carries
+    every owner's name, joined by '+' as in the app's parameter table.
+    """
+    found: dict[int, tuple[Parameter, str]] = {}  # first encounter, in walk order
+    owners: dict[int, list[str]] = {}
+
+    def walk(obj, owner_name: str, seen: set[int]):
         for _, child in paths.children(obj):
+            if id(child) in seen:
+                continue
+            seen.add(id(child))
             if isinstance(child, DescriptorNumber):
-                if isinstance(child, Parameter) and id(child) not in seen:
-                    seen.add(id(child))
-                    # A model's own parameter (scale) has the model as its owner: name it once.
-                    yield ' '.join(dict.fromkeys([model_name, owner_name, child.name])), child
-            elif id(child) not in seen:
-                seen.add(id(child))
-                yield from walk(child, model_name, getattr(child, 'name', owner_name))
+                if isinstance(child, Parameter):
+                    yield child, owner_name
+            else:
+                yield from walk(child, getattr(child, 'name', owner_name), seen)
 
+    # Each model is walked in full, shared objects included, to learn every owner.
     for model in project.models:
-        yield from walk(model, model.name, model.name)
+        for parameter, owner_name in walk(model, model.name, set()):
+            found.setdefault(id(parameter), (parameter, owner_name))
+            owners.setdefault(id(parameter), []).append(model.name)
+
+    for key, (parameter, owner_name) in found.items():
+        model_names = '+'.join(owners[key])
+        # A model's own parameter (scale) has the model as its owner: name it once.
+        yield ' '.join(dict.fromkeys([model_names, owner_name, parameter.name])), parameter
 
 
 def _truncate_name(name: str, max_len: int = _NAME_MAX_LEN) -> str:

tests/summary/test_summary.py

@@ -9,8 +9,11 @@
 from easyscience import global_object
 
 from easyreflectometry import Project
+from easyreflectometry.contrasts import ReplaceMaterial
 from easyreflectometry.model.resolution_functions import PercentageFwhm
+from easyreflectometry.sample import Material
 from easyreflectometry.summary import Summary
+from easyreflectometry.summary.summary import _named_parameters
 
 PATH_STATIC = os.path.join(os.path.dirname(os.path.dirname(__file__)), '_static')
 
@@ -351,3 +354,58 @@ def test_fit_experiment_figure_has_a_trace_per_channel(self, polarized_project:
         assert 'Experiment (pp)' in names
         assert 'Experiment (mm)' in names
         assert 'Model (pp)' in names
+
+
+class TestSummarySharedParameters:
+    @pytest.fixture
+    def contrast_project(self) -> Project:
+        """The default model plus an H2O contrast, which reuses the superphase and subphase assemblies."""
+        global_object.map._clear()
+        project = Project()
+        project.default_model()
+        d2o = project._materials[1]
+        project.add_contrast(0, 'H2O', [ReplaceMaterial(d2o, Material(sld=-0.56, isld=0, name='H2O'))])
+        return project
+
+    def test_shared_parameter_is_listed_once(self, contrast_project: Project) -> None:
+        # When
+        rows = list(_named_parameters(contrast_project))
+
+        # Then Expect: one row per distinct parameter object, none left out
+        ids = [id(parameter) for _, parameter in rows]
+        assert len(ids) == len(set(ids))
+        every = {id(parameter) for model in contrast_project.models for parameter in model.get_all_parameters()}
+        assert set(ids) == every
+
+    def test_shared_parameter_names_every_owner(self, contrast_project: Project) -> None:
+        # When
+        names = {id(parameter): name for name, parameter in _named_parameters(contrast_project)}
+        reference, contrast = contrast_project.models
+
+        # Then Expect: the shared subphase layer is attributed to both models, in model order
+        shared = reference.sample[2].layers[0].thickness
+        assert shared is contrast.sample[2].layers[0].thickness
+        assert contrast_project.parameter_models(shared) == [0, 1]
+        assert names[id(shared)] == 'Model+H2O Si Layer thickness'
+
+        # And an unshared parameter keeps its single model's name
+        own = reference.sample[1].layers[0].thickness
+        assert contrast_project.parameter_models(own) == [0]
+        assert names[id(own)] == 'Model D2O Layer thickness'
+
+    def test_owner_names_agree_with_project(self, contrast_project: Project) -> None:
+        # When
+        rows = list(_named_parameters(contrast_project))
+        models = contrast_project.models
+
+        # Then Expect: every row is prefixed by exactly the models the project says own it
+        for name, parameter in rows:
+            owners = '+'.join(models[index].name for index in contrast_project.parameter_models(parameter))
+            assert name.startswith(owners + ' ') or name == owners, name
+
+    def test_sample_section_shows_shared_owners(self, contrast_project: Project) -> None:
+        # When
+        html = Summary(contrast_project)._sample_section()
+
+        # Then Expect
+        assert 'Model+H2O Si Layer thickness' in html