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

sum and product models won't work with multiplicity (Trac #1022) #1089

Closed
pkienzle opened this issue Mar 30, 2019 · 13 comments · Fixed by #2647
Closed

sum and product models won't work with multiplicity (Trac #1022) #1089

pkienzle opened this issue Mar 30, 2019 · 13 comments · Fixed by #2647
Labels
Defect Bug or undesirable behaviour Major Big change in the code or important change in behaviour Multiplicity Issue related to handling of multiplicity
Milestone

Comments

@pkienzle
Copy link
Contributor

pkienzle commented Mar 30, 2019

See discussion in ticket #1075 and pull request [https://github.com/SasView/sasmodels/pull/53 #281].

This will also be a problem in mixture.py, if you are combining two models, one of which is a multiplicity model (combining two multiplicity models should raise an error). In that case, the control parameter will need to be tagged with the appropriate 'A_', 'B_', ... prefix before assigning, and the hidden() function, if present, will need to be wrapped so that the parameter names in the returned set are also prefixed.

Other parameters in model_info.ModelInfo should not be a problem. profile, profile_axes (for spherical shell models) are probably going to be drawn from the form_factor, and so don't need to be included. single and opencl are used internally to select a calculator, and can be undefined for this model (this model should present as a python model).

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

{
    "status": "new",
    "changetime": "2019-03-02T00:15:16",
    "_ts": "2019-03-02 00:15:16.703945+00:00",
    "description": "See discussion in ticket #1006 and pull request [https://github.com/SasView/sasmodels/pull/53 #53].\n\n\nThis will also be a problem in mixture.py, if you are combining two models, one of which is a multiplicity model (combining two multiplicity models should raise an error). In that case, the control parameter will need to be tagged with the appropriate 'A_', 'B_', ... prefix before assigning, and the hidden() function, if present, will need to be wrapped so that the parameter names in the returned set are also prefixed.\n\nOther parameters in model_info.ModelInfo should not be a problem. profile, profile_axes (for spherical shell models) are probably going to be drawn from the form_factor, and so don't need to be included. single and opencl are used internally to select a calculator, and can be undefined for this model (this model should present as a python model).\n",
    "reporter": "pkienzle",
    "cc": "",
    "resolution": "",
    "workpackage": "SasView Bug Fixing",
    "time": "2017-10-27T16:33:57",
    "component": "SasView",
    "summary": "sum and product models won't work with multiplicity",
    "priority": "major",
    "keywords": "",
    "milestone": "SasView 4.3.0",
    "owner": "",
    "type": "defect"
}
@pkienzle pkienzle added this to the SasView 4.3.0 milestone Mar 30, 2019
@pkienzle pkienzle added Defect Bug or undesirable behaviour Incomplete Migration Major Big change in the code or important change in behaviour and removed Incomplete Migration labels Mar 30, 2019
@butlerpd
Copy link
Member

Trac update at 2018/02/13 02:40:27: butler changed milestone from "SasView 4.2.0" to "SasView 4.3.0"

@pkienzle
Copy link
Contributor Author

Trac update at 2019/01/29 16:23:09: pkienzle commented:

A user wanted a fuzzy core-shell sphere plus a lorentzian. We did this by creating a sphere+lorentz model then edited the model, changing sphere to spherical_sld. The model works, but the user has to know to ignore the parameters for the extra shells.

Adapted model:


from sasmodels.core import load_model_info
from sasmodels.sasview_model import make_model_from_info

model_info = load_model_info('spherical_sld+lorentz')
model_info.name = 'snl'
Model = make_model_from_info(model_info)

@sasview-bot
Copy link

Trac update at 2019/01/29 16:26:35: Paul Kienzle pkienzle@nist.gov commented:

In changeset 15c80af4414c35749bdc01df46d248dfddd485d3:

#!CommitTicketReference repository="sasmodels" revision="15c80af4414c35749bdc01df46d248dfddd485d3"
propagate first multiplicity parameter to sum model. Refs #1089.

@sasview-bot
Copy link

Trac update at 2019/01/29 18:54:41: Paul Kienzle pkienzle@nist.gov commented:

In changeset b171acdab344b2827f9bdca29d66411c2484991f:

#!CommitTicketReference repository="sasmodels" revision="b171acdab344b2827f9bdca29d66411c2484991f"
move 'multiplicity' handling into sasview model. Refs #1089.

@sasview-bot
Copy link

Trac update at 2019/01/29 19:16:41: Paul Kienzle pkienzle@nist.gov commented:

In changeset 4f4d3e3574a18563fda8a9f5e84a9106aa7a1f1e:

#!CommitTicketReference repository="sasmodels" revision="4f4d3e3574a18563fda8a9f5e84a9106aa7a1f1e"
re-enable control parameter for rpa model. Refs #1089

@pkienzle
Copy link
Contributor Author

Trac update at 2019/01/29 19:17:50: pkienzle commented:

Sasmodels has been fixed.

Sasview needs to be updated so that the list of available models presented on the sum/multi box in sasview includes models with multiplicity.

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
Copy link
Member

Trac update at 2019/03/02 00:15:16: butler commented:

This is linked to #607 which is more the GUI side. When fully implemented this ticket should be closed.

@butlerpd butlerpd modified the milestones: SasView 4.3.0, SasView 5.1.0 Apr 26, 2020
@butlerpd
Copy link
Member

exposing the multiplicity models to the sum|multiply interface should be fairly simple. However refactoring the fitting GUI to handle a mixture model with two multiplicities is not obvious. Recommend we punt on doing this in a final 4.x release and move to 5.1

@wpotrzebowski
Copy link
Contributor

It still doesn't seem to work (from #2647):

#14:50:17 - ERROR: Fitting failed: Traceback (most recent call last): File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 79, in compute File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 19, in map_apply File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 16, in map_getattr File "sas/sascalc/fit/BumpsFitting.py", line 265, in fit File "bumps/fitproblem.py", line 203, in FitProblem File "bumps/fitproblem.py", line 634, in __init__ File "bumps/fitproblem.py", line 258, in model_reset ValueError: Need more data points than fitting parameters
14:50:17 - ERROR: Need more data points than fitting parameters None
14:50:17 - ERROR: Traceback (most recent call last): File "sas/qtgui/Perspectives/Fitting/FittingWidget.py", line 2053, in fitComplete AttributeError: 'str' object has no attribute 'fitness'
14:50:35 - ERROR: Fitting failed: Traceback (most recent call last): File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 79, in compute File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 19, in map_apply File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 16, in map_getattr File "sas/sascalc/fit/BumpsFitting.py", line 265, in fit File "bumps/fitproblem.py", line 203, in FitProblem File "bumps/fitproblem.py", line 634, in __init__ File "bumps/fitproblem.py", line 258, in model_reset ValueError: Need more data points than fitting parameters
14:50:35 - ERROR: Need more data points than fitting parameters None
14:50:35 - ERROR: Traceback (most recent call last): File "sas/qtgui/Perspectives/Fitting/FittingWidget.py", line 2053, in fitComplete AttributeError: 'str' object has no attribute 'fitness'

@krzywon
Copy link
Contributor

krzywon commented Nov 8, 2023

@wpotrzebowski, I just tested this locally using the branch in #2647. I added the core_multi_shell and the cylinder models together using Add/Multiply Models. I'm able to run fits and calculate theories using the resulting plugin model.

Can you walk me through the steps you took to get to the error you are seeing?

@wpotrzebowski
Copy link
Contributor

I think I tested wrong installer as I am not able to reproduce it - sorry.

@butlerpd
Copy link
Member

Does this mean this issue has been fixed @krzywon and @wpotrzebowski ? If so we can we close it? It is a rather old issue so nice to finally have it done 😄

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.
@butlerpd
Copy link
Member

butlerpd commented May 3, 2024

This should have been closed with the merging of #2647. Closing it manually.

@butlerpd butlerpd closed this as completed May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Bug or undesirable behaviour Major Big change in the code or important change in behaviour Multiplicity Issue related to handling of multiplicity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants