fix: allow constraints and restraints to be added after recipe is evaluated

news/fix-restraints.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* Apply restraints and constraints added to a ``FitContribution`` or ``ParameterSet`` after its ``FitRecipe`` has been evaluated. Before, they were ignored until the recipe was reset.
+
+**Security:**
+
+* <news item>

src/diffpy/srfit/fitbase/configurable.py

@@ -42,15 +42,24 @@ def _update_configuration(self):
         return
 
     def _store_configurable(self, obj):
-        """Store a Configurable.
+        """Store a Configurable to notify of configuration changes.
 
         The passed obj is only stored if it is a Configurable, otherwise
-        this method quietly exits.
+        this method quietly exits. Containers store themselves in the
+        objects they hold, so that changes are passed up the hierarchy.
         """
         if isinstance(obj, Configurable):
             self._configobjs.add(obj)
         return
 
+    def _remove_configurable(self, obj):
+        """Stop notifying a stored Configurable of changes.
+
+        The passed obj is ignored if it is not stored.
+        """
+        self._configobjs.discard(obj)
+        return
+
 
 # End class Configurable
 

src/diffpy/srfit/fitbase/recipeorganizer.py

@@ -628,15 +628,19 @@ def _add_object(self, obj, d, check=True):
         # Detach the old object, if there is one
         if oldobj is not None:
             oldobj.remove_observer(self._flush)
+            if isinstance(oldobj, Configurable):
+                oldobj._remove_configurable(self)
 
         # Add the object
         d[obj.name] = obj
 
         # Observe the object
         obj.add_observer(self._flush)
 
-        # Store this as a configurable object
-        self._store_configurable(obj)
+        # Pass configuration changes in the object up to this one
+        if isinstance(obj, Configurable):
+            obj._store_configurable(self)
+            self._update_configuration()
         return
 
     def _remove_object(self, obj, d):
@@ -653,6 +657,9 @@ def _remove_object(self, obj, d):
 
         del d[obj.name]
         obj.remove_observer(self._flush)
+        if isinstance(obj, Configurable):
+            obj._remove_configurable(self)
+            self._update_configuration()
 
         return
 

src/diffpy/srfit/structure/objcrystparset.py

@@ -358,15 +358,15 @@ def wrapRestraints(self):
         # from within this class.
         for b in self.scat.GetBondList():
             res = ObjCrystMoleculeRestraint(b)
-            self._restraints.add(res)
+            self.register_soft_bounds(res)
 
         for ba in self.scat.GetBondAngleList():
             res = ObjCrystMoleculeRestraint(ba)
-            self._restraints.add(res)
+            self.register_soft_bounds(res)
 
         for da in self.scat.GetDihedralAngleList():
             res = ObjCrystMoleculeRestraint(da)
-            self._restraints.add(res)
+            self.register_soft_bounds(res)
 
         return
 
@@ -463,7 +463,7 @@ def restrainBondLength(
         res = ObjCrystBondLengthRestraint(
             atom1, atom2, length, sigma, delta, scaled
         )
-        self._restraints.add(res)
+        self.register_soft_bounds(res)
 
         return res
 
@@ -537,7 +537,7 @@ def restrainBondAngle(
         res = ObjCrystBondAngleRestraint(
             atom1, atom2, atom3, angle, sigma, delta, scaled
         )
-        self._restraints.add(res)
+        self.register_soft_bounds(res)
 
         return res
 
@@ -612,7 +612,7 @@ def restrainDihedralAngle(
         res = ObjCrystDihedralAngleRestraint(
             atom1, atom2, atom3, atom4, angle, sigma, delta, scaled
         )
-        self._restraints.add(res)
+        self.register_soft_bounds(res)
 
         return res
 

tests/test_fitrecipe.py

@@ -32,6 +32,7 @@
     ProfileParser,
 )
 from diffpy.srfit.fitbase.parameter import Parameter
+from diffpy.srfit.fitbase.parameterset import ParameterSet
 
 matplotlib.use("Agg")
 
@@ -327,6 +328,104 @@ def test_residual_with_contribution_constraints_and_restraints(
     assert actual_final_residual == pytest.approx(expected_final_residual)
 
 
+def _get_restraint_bounds(restraints):
+    """Return the sorted (lower_bound, upper_bound, sig) of each
+    restraint."""
+    sorted_restraints = sorted(
+        (restraint.lower_bound, restraint.upper_bound, restraint.sig)
+        for restraint in restraints
+    )
+    return sorted_restraints
+
+
+def test_add_bounds_after_recipe_evaluation():
+    # Test that soft bounds added to a contribution after the recipe is
+    # evaluated are used in the next evaluation.
+    profile = Profile()
+    profile.set_observed_profile(linspace(0, 1, 5), ones(5))
+    contribution = FitContribution("contrib")
+    contribution.set_profile(profile)
+    contribution.set_equation("m*x + b")
+    # C1: m is restrained in the contribution before it is added to a recipe.
+    # Expected: The contribution holds the restraint on m.
+    contribution.add_soft_bounds("m", lower_bound=0, upper_bound=2, sig=1)
+    actual_bounds_before_eval = _get_restraint_bounds(
+        contribution._get_restraints()
+    )
+    expected_bounds_before_eval = [(0, 2, 1)]
+    assert actual_bounds_before_eval == expected_bounds_before_eval
+    # C2: The recipe is evaluated, then b is restrained in the contribution
+    # and the recipe is evaluated again.
+    # Expected: The recipe uses the restraints on both m and b.
+    recipe = FitRecipe()
+    recipe.add_contribution(contribution)
+    recipe.add_variable(contribution.m, 1)
+    recipe.add_variable(contribution.b, 0)
+    # Evaluate the recipe.
+    recipe.residual()
+    recipe.contrib.add_soft_bounds("b", lower_bound=-1, upper_bound=1, sig=3)
+    # Evaluate the recipe again to include the new restraint.
+    recipe.residual()
+    actual_bounds_after_eval = _get_restraint_bounds(recipe._restraintlist)
+    expected_bounds_after_eval = [(-1, 1, 3), (0, 2, 1)]
+    assert actual_bounds_after_eval == expected_bounds_after_eval
+
+
+def test_add_bounds_to_nested_parameter_set_after_recipe_evaluation(
+    build_sine_recipe,
+):
+    # Test that soft bounds added to a ParameterSet nested two levels below
+    # the recipe after the recipe is evaluated are used in the next
+    # evaluation.
+    outer = ParameterSet("outer")
+    inner = ParameterSet("inner")
+    inner.new_parameter("p", 1)
+    inner.new_parameter("q", 1)
+    # C1: p is restrained in "inner" before "inner" is added to a recipe.
+    # Expected: "inner" holds the restraint on p.
+    inner.add_soft_bounds("p", lower_bound=0, upper_bound=2, sig=1)
+    actual_bounds_before_eval = _get_restraint_bounds(inner._get_restraints())
+    expected_bounds_before_eval = [(0, 2, 1)]
+    assert actual_bounds_before_eval == expected_bounds_before_eval
+    # C2: The recipe holding "outer", which holds "inner", is evaluated, then
+    # q is restrained in "inner" and the recipe is evaluated again.
+    # Expected: The recipe uses the restraints on both p and q.
+    recipe = build_sine_recipe()
+    outer.add_parameter_set(inner)
+    recipe.add_parameter_set(outer)
+    # Evaluate the recipe.
+    recipe.residual()
+    recipe.outer.inner.add_soft_bounds(
+        "q", lower_bound=-1, upper_bound=1, sig=3
+    )
+    # Evaluate the recipe again to include the new restraint.
+    recipe.residual()
+    actual_bounds_after_eval = _get_restraint_bounds(recipe._restraintlist)
+    expected_bounds_after_eval = [(-1, 1, 3), (0, 2, 1)]
+    assert actual_bounds_after_eval == expected_bounds_after_eval
+
+
+def test_add_constraint_after_recipe_evaluation(build_sine_recipe):
+    # Test that a constraint added to a contribution after the recipe is
+    # evaluated is updated in the next evaluations.
+    recipe = build_sine_recipe()
+    recipe.add_variable(recipe.cont.A)
+    # Evaluate the recipe.
+    recipe.residual()
+    # C1: c is constrained to 2*A in the contribution with A = 1.
+    # Expected: c is 2.
+    recipe.cont.add_constraint(recipe.cont.c, "2*A")
+    actual_c_value = recipe.cont.c.value
+    expected_c_value = 2
+    assert actual_c_value == expected_c_value
+    # C2: The recipe is evaluated again with A = 3.
+    # Expected: c follows A and is 6.
+    recipe.residual([3])
+    actual_c_value = recipe.cont.c.value
+    expected_c_value = 6
+    assert actual_c_value == expected_c_value
+
+
 def test_convert_bounds_to_restraints():
     # C1: A variable bounded to [-1, 1] is converted with sig=2 and
     # scaled=True.