SasView / sasview

Code for the SasView application.
BSD 3-Clause "New" or "Revised" License
49 stars 41 forks source link

Theoretical calculations with constrained fit #1741

Open YuyinXi opened 3 years ago

YuyinXi commented 3 years ago

I am using the SasView version 5.0.3. The constrained fitting works for me, but the theoretical calculation does not take in the constraints. In other words, when the free varying parameter is manually changed, the customized constrain equation won't automatically apply to the model calculation. For example, if I have a constraint of scale_A=1-scale_B, by manually changing scale_B to 0.2, the calculation won't plug in a corresponding scale_A value of 0.8 in the model.

RichardHeenan commented 3 years ago

Have tried to have this implemented previously, but it may not be easy as constraints are handled by the fit engine. However the current behaviour is inconsistent as changing the value of a parameter does normally update I(Q).

(Note that following earlier discussion the constrained parameter shows as "on or adjusting" and is in blue italic)

gonzalezma commented 8 months ago

I agree that this would be a nice thing to have, but it seems to me far from easy to implement (@rozyczko What do you think?). As Richard indicated, the constraints are handled by bumps, while the Compute/Plot calls the model with the parameters in the FitPage window. I think that to implement this, one would need to add extra links between the FitPages and the ConstraintsPage, so that every time that a constraint is defined or modified, the corresponding constrained parameters are recomputed. This should be possible, but is it worthy the effort? As we are not fitting, but just calculating a theoretical curve, I would claim that it is up to the user to fill in the right values. Should we close this?

bmaranville commented 8 months ago

I tried to figure out how Parameter constraints are being applied, and at least within sasview it appears to be through a setp_hook function that overrides Parameter values with the constraint expressions provided, as part of the fit() function https://github.com/SasView/sasview/blob/8ea68655c7c58e01efe264cdaf59d3afda161939/src/sas/sascalc/fit/BumpsFitting.py#L272 The modification applied seems to work through an eval() statement that modifies the problem object namespace (context) directly, inserting some new names and definitions to implement the constraints.

@pkienzle suggested (implemented) a new mechanism for applying inter-parameter equality constraints (which seem to be the only type of constraints used in sasview?) by adding an abstraction layer so that all Parameter objects have a "slot" attribute which can contain:

  1. a very simple Variable object (class with single "value" attribute that is a float)
  2. another Parameter object
  3. an (arbitrarily complex) Expression object (with attributes for the expression arguments and operation)

All of these have a "value" property or attribute that allows them to be evaluated within the model, by calling Parameter.value This change would make adding the type of constraints used in sasview pretty simple (scale_A = 1 - scale_B becomes scale_A.slot = Expression(op="sub", args=(1, scale_B)). New Parameter objects directly instantiated have a Variable object in their "slot" attribute, with the value provided to the init function. You would still have to make sure you don't introduce circular parameter dependencies, but there is already code in sasview to check that.

The new implementation can be found in the dataclass_overlay or webview branches, which we hope to merge this year sometime.

EDIT: note that the reason I mention this here on this ticket is that if you were handling constraints in this way, it can be done independently of any fitting, as long as the parameters in the problem are instances of bumps.parameter.Parameter. Then any calculated theoretical curves would be consistent with fit results as the constraints would be defined within the problem directly.

gonzalezma commented 8 months ago

This could be a way forward. A similar request to the current issue was already reported in #569.