Various Multiplicity Model Fixes

[krzywon]

Description

This fixes a number of multiplicity issues including addition of models in the sum|multi custom combo boxes, copy/paste of parameters, copy to excel/latex files, linking to structure factors, and suppressing the SLD profile button for models without SLD parameters.

Fixes #1089
Fixes #1897
Fixes #2369
Fixes #2563

How Has This Been Tested?

Tested locally using the core_multi_shell, onion, and unified_power_Rg built-in models as well as the core_multi_shell_cylinder model from sasmodels->newmodles0923.

Review Checklist (please remove items if they don't apply):

• Code has been reviewed
• Functionality has been tested
• Windows installer (GH artifact) has been tested (installed and worked)
• MacOSX installer (GH artifact) has been tested (installed and worked)
• User documentation is available and complete (if required)
• Developers documentation is available and complete (if required)
• The introduced changes comply with SasView license (BSD 3-Clause)

[rozyczko]

Code looks good. Basic functionality checked. Multishell models serialize and deserialize properly.
Response to the shell combobox is as expected but we still lose the previously entered values on shell change. Should this be addressed? I believe it was mentioned as a bug in one of the issues.

[RichardHeenan]

Built locally using 2369-multiplicity-copy and newmodels0923, I am happy so far, well done Jeff for fixing this.

core-multi-shell with one shell now gives same fitting result as core-shell-sphere, including polydisp on core radius and hard sphere S(Q)

core-multi-shell-cylinder with one shell now gives same fitting result as core-shell-cylinder, including with S(Q), and with reasonably close results with 3 or 4 polydispersity distributions (though understandably a bit slow to compute)

[ Also - when I have the same data in more than one fit tab and use S(Q) the saving of P(Q) and S(Q) in the data explorer replaces previous one from another tab, which it ought not to.]

[Also there the new dropdown in "send to" for "replace" data in current fit tab is not working for me, the dropdown has only the one choice]

[lucas-wilkins]

@RichardHeenan

[Also there the new dropdown in "send to" for "replace" data in current fit tab is not working for me, the dropdown has only the one choice]

The dropdown does replace if you select the only option, and normal send to if you just click the main button.

[krzywon]

To answer the question at today's call by @butlerpd, this does not fix the issue related to the RPA model.

[RichardHeenan]

I need to find some time to do some more testing, and also to merge my newmodels0923 branches with more recent changes.

One of my new models, the lamellar_x5_ one, which is not a multiplicity model, is prone to giving overflows & underflows, especially if using L-M fitting. When this happens the I(Q) in the data explorer comes back all as nan or inf. In my local build it seems that (a) no error messages appear anywhere, (b) the model (or perhaps rather the comms with the gui) seems then to be corrupted, failing to work properly from that point onward, even with calculations that I know ought to work. Testing simple forwards calculations of the I(Q) from the model using the "compare" py script suggests the model itself is behaving correctly. Need to to some more testing, perhaps make a temporary model that can be deliberately failed by a change of parameter value.

UPDATE - 13h Oct. Having pulled latest changes to the sasview part of my local win build, I am fairly convinced that the odd issues with lamellar_x5 are likely due to the imposed zero to inf range limit on the parameter values. I have still all zero I(Q) returning some times, but starting from a different set of parameter values the calculations and fits then work OK. A bit odd, but paracrystal models are always rather picky, I will now do some more testing on an actual multiplicity model.

[butlerpd]

My concern about the RPA not working is whether this is because the fix is not really correct, i.e. the special parameter is not being interpreted as defined. If however it is just a case of extra code needed to fully implement the use of the special parameter I'm good with calling this an improvement and move forward approving. @krzywon has agreed to look into this.

[gonzalezma]

I don't know if this is unrelated to this branch, but I had to add an 'import subprocess' to the file convertUI.py to make it work.

[krzywon]

I don't know if this is unrelated to this branch, but I had to add an 'import subprocess' to the file convertUI.py to make it work.

The latest merge commit should fix this.

One is that if one creates a mixture model where one of the models is a multiplicity model, the 'Show SLD Profile' button appears in the plugin model. However, when you click the button an error occurs:
'''
ERROR: SLD profile calculation failed.
'''

This is one of the items I was going to ticket from #1089. It's apparently been around for a while.

The second one is that changing the value of n resets all the "multiplicity" values to the default. For example, if I first do a fit with core_multi_shell and n=1 and then change to n=3, the previous values for the parameters sld_core, sld_solvent and radius are conserved, but sld1 and thickness1 not. This is more annoying in the polydispersity tab, if one has previously changed any of the distributions from gaussian to e.g. lognormal or schulz, as shown in the image:

Very annoying. I'm looking into this now.

[gonzalezma]

@smk78 just reported a similar problem in a comment to issue #2438, so perhaps this issue is not directly related to the changes here?

[krzywon]

Final update:

changing the value of n resets all the "multiplicity" values to the default.

This should now be fixed using a simple copy-paste of parameters.

@smk78 just reported a similar problem in a comment to issue #2438, so perhaps this issue is not directly related to the changes here?

I can reliably recreate this in v5.0.6 (and maybe spent too much time trying to debug this). It appears to be related to simultaneous signals that cause conflicting behavior. I'll add more in the issue, but this has been around a while, is not related to this PR, and could be a complex fix.

[krzywon]

For clarity during a review, here are the issues this PR fixes or improves:

• Copy params to latex/excel formats now works for multiplicity models.
• SLD profile button is suppressed for models with no SLD parameters.
• Changing the number of layers in a multiplicity model retains any modified values.
• Polydispersity distribution types are now available for layer parameters.
• P*S models no longer throw errors when changing multiplicity parameters.
• The RPA model is no longer available in the add/multi model editor window.
• Multiplicity models can no longer be combined using the easy add/multiply editor.
• Documentation changes
  • Differentiate @ vs *
  • Suppression of layered models if one already selected
• Polydispersity tab no longer accessible with checkbox unchecked.
• Removed a number of if not dict: pass statements (they do nothing).

And issues this PR does not fix:

• PD distribution combobox sometimes ends up in the filename column - issue since v5.0.6 (at least)
• Changing the number of layers in a multiplicity model will reset PD params - Polydispersity settings not updating correctly #2438
• Combining two multiplicity models - issue since v4.0.0
• RPA (anything related to this) - issue since v5.0.0
• SLD Calculation failures - issue since ?

[butlerpd]

Everything that this PR claims to fix (which is not everything to do with multiplicity) seems to work as advertised save one thing which is still mostly fixed. However this is a great improvement over 5.x so the issues noted below should not prevent merging as necessary. That said here is what I found on windows 10:

• A sum model that contains a layered model with no SLD does NOT suppress the SLD profile which will throw an error if pressed. The suppression does seem to work in all other cases.
• Loading a multi SLD (but not a multiplicity that does not contain SLD such as the unified_power_Rg model) throws a warning pop-up saying “Clipboard content is incompatible with the fit page.” This also happened after playing with the core_mutli_shell model and then adding a HS structure factor. As soon as the structure factor was chosen the pop-up occurred. It is almost as if choosing a model when it is an SLD profile model automatically tries to get something from the clipboard?
• Finally, while the Copy parameters seems to work (as does Copy paste from a multiplicity model to another model), I notice that the table created includes extraneous parameters like the name of the model (andmultiplicity in the case it is) This is clearly not part of this PR but is annoying I think?
• The other part of the Copy parameters which again is not likely to be part of the PR is that the TRUE FALSE flags for whether a parameter is being fit do NOT align. That seems to be true regardless of multiplicity or not.

[krzywon]

A sum model that contains a layered model with no SLD does NOT suppress the SLD profile which will throw an error if pressed. The suppression does seem to work in all other cases.

This is now fixed. I don't plan to fix the other issues noted here in this PR.

[butlerpd]

This appears to fix both the first AND second points above. Not sure why? but I'll take it.

However .... I do not understand why the onion model does not show the SLD. The reg expression should identify it and the model does contain the SLD function (def profile). I know I tested at least one of core_multi_shell and onion in the previous tests but maybe I forgot to test onion specifically (the core_multi_shell does in fact show the SLD profile button)? Again I would not hold up merging for this.

[butlerpd]

Right ... color me totally confused. This final fix now allows the onion model's SLD profile to be shown along with others and now that SLD profile is suppressed for non SLD multiplicity models even in sum/multiply, the show profile button still throws an error for the mixed models that do have an SLD profile.

 ERROR: SLD profile calculation failed.

I thought we checked that and it was working before? but looking at the code I'm not sure that is possible. It is the same error reported by @gonzalezma earlier and seems to come from the call to onShowSLDProfile(self) try except block aroun

def onShowSLDProfile(self):
    """
    Show a quick plot of SLD profile
    """
    # get profile data
    try:
        x, y = self.kernel_module.getProfile()
    except TypeError:
        msg = "SLD profile calculation failed."
        logging.error(msg)
        return

Should we just suppress the profile for all mixture models?

On the other hand, I think all the issues identified by @smk78 which led him to request changes have been addressed from my testing so that block on merging should be removed I think.

[krzywon]

The SLD calculation error is one of the items I listed as 'not going to fix' in this PR. It's present in v5.0.6, so not something I introduced here.

[butlerpd]

Fair enough - Pencils down you are telling me :-). I was wondering about just disabling the button for all mixture models but then maybe that will just make us forget about the problem (out of sight out of mind?). In which case we should be ready to merge IMO