SasView / sasview

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

QT GUI - complex constraints (master ticket) (Trac #1135) #1376

Closed sasview-bot closed 4 years ago

sasview-bot commented 5 years ago

In the QT GUI, there are some issues with the complex constraints functionality.

  1. Activating the complex constraints dialog is only possible by selecting multiple fit pages in a special table before right-clicking to bring up a context menu.
  2. The dialog has a text box for editing constraints, but it breaks when using multiple parameters on the right-hand side.
  3. The dialog does not allow you to select which models to take fitting parameters from; it is fixed to whichever two fit pages were selected prior to activating the dialog.

Possible solutions to the above problems are, respectively:

  1. Add a simple button labelled "Add Constraint" or similar.
  2. Fix the text box!
  3. Add combo boxes (drop-downs) for the left- and right-hand sides of the constraint, to allow a model to be chosen before choosing a parameter from that model.

I have some agreement with Richard and Piotr on this, so far, and will add updates to this ticket in the next few days.

(I'm putting this in the beta approx. work package but feel free to move it, I'm not sure where it should go...!)

Migrated from http://trac.sasview.org/ticket/1135

{
    "status": "new",
    "changetime": "2018-07-23T09:42:05",
    "_ts": "2018-07-23 09:42:05.573581+00:00",
    "description": "In the QT GUI, there are some issues with the complex constraints functionality.\n\n 1. Activating the complex constraints dialog is only possible by selecting multiple fit pages in a special table before right-clicking to bring up a context menu.\n 1. The dialog has a text box for editing constraints, but it breaks when using multiple parameters on the right-hand side.\n 1. The dialog does not allow you to select which models to take fitting parameters from; it is fixed to whichever two fit pages were selected prior to activating the dialog.\n\nPossible solutions to the above problems are, respectively:\n\n 1. Add a simple button labelled \"Add Constraint\" or similar.\n 1. Fix the text box!\n 1. Add combo boxes (drop-downs) for the left- and right-hand sides of the constraint, to allow a model to be chosen before choosing a parameter from that model.\n\nI have some agreement with Richard and Piotr on this, so far, and will add updates to this ticket in the next few days.\n\n\n(I'm putting this in the beta approx. work package but feel free to move it, I'm not sure where it should go...!)",
    "reporter": "tcbennun",
    "cc": "piotr, wojciech, richardh",
    "resolution": "",
    "workpackage": "Beta Approximation Project",
    "time": "2018-07-18T16:14:13",
    "component": "SasView",
    "summary": "QT GUI - complex constraints (master ticket)",
    "priority": "blocker",
    "keywords": "",
    "milestone": "SasView 5.0.0",
    "owner": "tcbennun",
    "type": "defect"
}
rozyczko commented 5 years ago

Trac update at 2018/07/19 09:36:27: piotr commented:

Issues 2. and 3. are both based on the initial decision to support two models for intermodel constraints. If the requirement is to include more than 2 models in the complex constraint functionality, the design will have to be redone.

I agree with 1., as discussed with Torin elsewhere.

sasview-bot commented 5 years ago

Trac update at 2018/07/19 09:38:32: tcbennun changed milestone from "SasView 4.2.0" to "SasView 5.0.0"

sasview-bot commented 5 years ago

Trac update at 2018/07/19 11:01:40:

Replying to [comment:1 piotr]:

Issues 2. and 3. are both based on the initial decision to support two models for intermodel constraints. If the requirement is to include more than 2 models in the complex constraint functionality, the design will have to be redone.

I agree with 1., as discussed with Torin elsewhere.

For 3. it still only uses 2 models, it would just be nice to have a 'generic' dialog box in which you can select the 2 models you want to use.

Re issue 2., even if you use 2 parameters from the same model on the right-hand side it breaks, e.g. {{M1.radius < M2.radius + M2.thickness}} is invalid, even though parameters from only 2 models are being used.

to:

1531998117629910

Replying to [comment:1 piotr]:

Issues 2. and 3. are both based on the initial decision to support two models for intermodel constraints. If the requirement is to include more than 2 models in the complex constraint functionality, the design will have to be redone.

I agree with 1., as discussed with Torin elsewhere.

For 3. it still only uses 2 models, it would just be nice to have a 'generic' dialog box in which you can select the 2 models you want to use.

Re issue 2., even if you use 2 parameters from the same model on the right-hand side it breaks, e.g. M1.radius < M2.radius + M2.thickness is invalid, even though parameters from only 2 models are being used.

rozyczko commented 5 years ago

Trac update at 2018/07/19 12:09:05: piotr commented:

Replying to [comment:3 tcbennun]:

For 3. it still only uses 2 models, it would just be nice to have a 'generic' dialog box in which you can select the 2 models you want to use.

Hmm One potential scenario this could be useful is when there are many model pages displayed and the user wants to navigate between them without having to ctrl-click in the list potentially making mistakes by mis-clicking. Maybe then the solution would be to use just one widget which would have a checkbox for using selected model pages (if 2 selected) vs using combo boxes as you described above? So for any other selection you would have a dialog with only:

<model 1 combo> <param 1 combo> <operand> <model 2 combo> <param 2 combo>

with the checkbox disabled. For 2 selected models you would have the above, when the checkbox is off and the current setup when the checkbox is on

<model 1 label> <param 1 combo> <operand combo> <model 2 label> <param 2 combo>

This is much less confusing than separate dialogs with different content.

Re issue 2., even if you use 2 parameters from the same model on the right-hand side it breaks, e.g. M1.radius < M2.radius + M2.thickness is invalid, even though parameters from only 2 models are being used.

Again, wrong initial assumption on what the requirements were. Yes - this needs to be fixed.

sasview-bot commented 5 years ago

Trac update at 2018/07/23 08:32:51: tcbennun commented:

Sounds good!

This is kinda just details, but I was also thinking that the ctrl-click interface is possibly a little unnecessary. Maybe simply clicking a fit page in the list could select it, while retaining the previous selection. (Also might be necessary to forcibly limit the number of selected fit pages to two, i.e. if two are already selected, selecting another one would automatically deselect the 'oldest' selected page. Currently if you select more than two fit pages, the dialog simply doesn't appear.)

rozyczko commented 5 years ago

Trac update at 2018/07/23 08:40:29: piotr commented:

Replying to [comment:5 tcbennun]:

Sounds good!

Also might be necessary to forcibly limit the number of selected fit pages to two, i.e. if two are already selected, selecting another one would automatically deselect the 'oldest' selected page.

I would like to retain multiple selections for the "Select/deselect pages for fitting" functionality. The additional button discussed earlier (for defining constraints) would get enabled only when two pages were selected and a tooltip over the button could explain its enablement criteria.

sasview-bot commented 5 years ago

Trac update at 2018/07/23 08:42:55: tcbennun commented:

Replying to [comment:6 piotr]:

I would like to retain multiple selections for the "Select/deselect pages for fitting" functionality. The additional button discussed earlier (for defining constraints) would get enabled only when two pages were selected and a tooltip over the button could explain its enablement criteria. Ahh, I see now, thanks!

RichardHeenan commented 5 years ago

Trac update at 2018/07/23 09:42:05: richardh commented:

OK to summarise, there are two functionalities which both involve selecting Fitpages, which both have to be obvious to the user:

(a) which of possibly many Fitpages are to be included in the simultaneous fit when they hit the Fit button on the simultaneous fit tab.

(b) which two fitpages are involved in a constraint that a user wants to set up. [ Note, due to horrible issues with polydispersity integrals we do not allow a constraint within a single Fitpage, and suggest instead that the user writes a custom version of the model to acheive this.]

Personally I like the idea that by use of drop downs etc an equation appears in an editable box e.g.

M1.radius < M2.radius + M2.thickness

which can then be further customised by the user e.g to become

M1.radius < 0.75*M2.radius + 3.0*M2.thickness

with an "OK" or "add this constraint" button when finished editing.

[Note, suspect inequality constraints need some further work in fitting engines, so should not actually appear in 5.0, see #560]

RichardHeenan commented 5 years ago

A few of us ought to test constraints in sasview 5, both within a single fit page and between fit pages when fitting simultaneous data sets.

Then the new system will need some documentation.

RichardHeenan commented 5 years ago

1) I think there ought to be a "constraints" button somewhere on the fitpage that says how to introduce one (see if you can work out how to make one within a single fitpage).

2) Though the constrained parameter name goes into blue italic, perhaps it should also have say a yellow background to make it even more obvious?

3) If possible, having removed a constraint, could we have an option to "reinstate previous constraint"

4) Note constraints are only applied if the constrained parameter is "on" and you have hit "fit". I think that entering a new parameter value elsewhere or forcing a compute with showplot should also call the constraints.

5) I have not yet tested whether save & load project keeps constraints. - Seems to work, but see issue 9 below.

6) You will have noted frequent messages warning if you constrain a potentially polydisperse parameter. See here for one possible way around this: https://github.com/SasView/sasmodels/issues/215#issue-431241432 sorry meant https://github.com/SasView/sasview/issues/501

[ASIDE - I am also curious as to how sasview appiles the constraints when doing e.g. L-M fit, does it find the derivatives of I(Q) for all parameters involved, then tie the derivatives, or does it apply the constraints after shifting a parameter value to find that derivative of I(Q) in one shot? ]

RichardHeenan commented 5 years ago

Here is a project with core, drop & shell microemuslions contrasts, and polydispersity on some of the radii, rename .txt to .json 3sets_5constraints_polydisp_json.txt

The chi2 values should be 42.163, 52.486, 24.009 respectively. Continuing previous list of requests & observations:

  1. Polydispersity parameters are not appearing in the constraints editor, they ought to be there.

  2. The editor has a box around "2 parameter constraints", that box really only applies to the drop downs part, as the free text line underneath can be edited for e.g. M2.radius = M3.radius + M3.thickness as per the example project, Perhaps insert a label above the free text box to say "Edit complex constraint:" or similar.

  3. If you open the project a second time, sasview says it is going to remove all current plots, but does not actually do so, fitpages 1 to 3 do however go, and the new open project uses fitpages 4, 5 & 6. However on the constraints tab the mnemonic for the third set is wrong, it has M1,M2,M6 instead of M1,M2,M3. Opening the project a third time gives M1,M2,M9. The simultaneous fit then behaves very strangely and is incorrect. (.e.g. though hitting "show plot" on the individual page may give the correct result, try setting scale on that page way off, it does not adjust back again when running the simultaneous fit, nor are paremeter error values appearing, so I think that the constraints are in a mess.) From the user perspective it would be more helpful if sasview re-used fitpages 1,2,3 after deleting them, rather than changing to 4,5 & 6.

  4. As noted here for 4.2, https://github.com/SasView/sasview/issues/1117 the errors on constrained parameters are appearing as nan, which perhaps they ought not to?

pkienzle commented 5 years ago

[ASIDE - I am also curious as to how sasview appiles the constraints when doing e.g. L-M fit, does it find the derivatives of I(Q) for all parameters involved, then tie the derivatives, or does it apply the constraints after shifting a parameter value to find that derivative of I(Q) in one shot? ]

L-M finds Jacobian using numerical differentiation at every Q point independently, each wrt to the fitted parameters, whatever they are. That is to say, the constraints are automatically included in the derivative calculation. Similarly for DREAM: the constraints are part of the model, and so their effects are included in the posterior distribution. - Good, as I expected, thank you!

pkienzle commented 5 years ago

Here is a project with core, drop & shell microemuslions contrasts, and polydispersity on some of the radii, rename .txt to .json 3sets_5constraints_polydisp_json.txt

Don't know where this JSON is coming from (some sort of json pickler?), but you may want to set it so that it saves numpy array data as lists of numbers rather than binary. That will allow you to process the data using a variety of JSON tools and languages rather than specifically python with numpy.

RichardHeenan commented 5 years ago

Here is a project with core, drop & shell microemuslions contrasts, and polydispersity on some of the radii, rename .txt to .json 3sets_5constraints_polydisp_json.txt

Don't know where this JSON is coming from (some sort of json pickler?), but you may want to set it so that it saves numpy array data as lists of numbers rather than binary. That will allow you to process the data using a variety of JSON tools and languages rather than specifically python with numpy.

The json is of course a "save project" from 5.0 - suggest we ask Piotr & Wojtek. However the messy binary is some meta data, not used in sasview, specifically here for tof transmissions, as pulled in from the cansas format xml data file, see for example AOT_Microemulsion-Core_contrast.xml in the sasview test data folder.

RichardHeenan commented 4 years ago

Closing this as out of date now, need to test constrained multi-set projects again.

RichardHeenan commented 4 years ago

See #1588 mentioned above for a more up to date test data set.