Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiplicity (and control parameter) handling #2243

Open
butlerpd opened this issue Oct 10, 2022 · 3 comments
Open

Multiplicity (and control parameter) handling #2243

butlerpd opened this issue Oct 10, 2022 · 3 comments
Assignees
Labels
Code Camp 2024 Critical High priority Defect Bug or undesirable behaviour Multiplicity Issue related to handling of multiplicity
Milestone

Comments

@butlerpd
Copy link
Member

butlerpd commented Oct 10, 2022

Describe the bug
sasmodels implements control parameters which are hidden. They are documented (lightly) in the developer docs, in particular at:
https://www.sasview.org/docs/dev/sasmodels-api/sasmodels.html#submodules

However, the GUI does not interpret/use them correctly. These are used in the so-called multiplicity models which includes the RPA, the core_multishell, the unified_power_Rg, and the onion models. The RPA is different from the others in that it is really a series of cases rather than a number of shells. While there are issues with all, the RPA is so broken in the GUI that it has been removed form version 5.

Someone needs to relook at the GUI interface to sasmodels with an eye to properly working with control parameters.

This is essentially the issue reported in sasmodels: SasView/sasmodels#396. That really should be a sasview issue as it is a GUI issue so linking that discussion here but will close that issue.

Expected behavior
It should work (give the correct answers)

SasView version (please complete the following information):

  • certainly all of 5.x though there are suggestions that RPA may not have worked even in 4.x

Operating system (please complete the following information):

  • OS: all

Additional context
Some other related sasview issues are: #607, #965, #1089 (which seems to be the same as #607), #1106, #1816. Note that some of these may have been fixed without closing the issue?

@RichardHeenan
Copy link
Contributor

Additionally, adding S(Q) to say core_multi_shell does not give the correct results, nor does it give any error messages to the user. The attached project file will show this. The fits on the two plots below should, if I got the models right, be the same!

The core_multi_shell is not adding separate P(Q) and s(Q) to the data explorer.

More worrying still, if you remove the S(Q) and then fit just "scale" the core_multi_shell still does not give quite the same result as core_shell_sphere (no plots here). How different they are depends on the value of the flat background. I suspect that for core_multi_shell the flat background is perhaps not being included in the chi squared (though is there in the I(Q) on the plot).

Basically the gui seems to have a lot of issues with multiplicity models, which will not be apparent to the average user.

Screenshot 2023-09-19 114154

Screenshot 2023-09-19 114532
Screenshot 2023-09-19 114614

test_multishellSQ_19Sept2023_506release.json.txt

@smk78 smk78 added the Multiplicity Issue related to handling of multiplicity label Sep 21, 2023
@smk78
Copy link
Contributor

smk78 commented Jan 22, 2024

As far as the rpa model is concerned, the consensus at the 2024 Contributor Camp roadmap discussion was that this model should not be re-implemented in the form it once was (ie, using a control variable to select the calculation case). Instead, each case should be implemented as a separate model.

@butlerpd
Copy link
Member Author

butlerpd commented May 2, 2024

In closing issue #1089 with the merging of PR #2647, @pkienzle's comment about replacing the old multiplicity concept being maintained in sasview with sasmodels' more sophisticated control and hidden parameters is copied over here.

Could redo the sasview model table view so that parameter list is updated whenever any control parameter is updated (sasmodels parameters each have an "is_control" attribute), and drop the notion of a "multiplicity" model.

butlerpd added a commit that referenced this issue May 3, 2024
Various Multiplicity Model Fixes
Merging as agreed. Much improved multiplicity behavior and not time to put our pencils down. I have moved the comment by @pkienzle in #1089 which is being closed by this merge to #2243 for safe keeping. Created a new ticket, #2866 about SQ causing a reset of multiplicity parameters and #2867 about the error reported above about the SLD profile button causing an error when it is part of a mixture model with one SLD multiplicity model.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Camp 2024 Critical High priority Defect Bug or undesirable behaviour Multiplicity Issue related to handling of multiplicity
Projects
None yet
Development

No branches or pull requests

4 participants