Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#1035 closed defect (fixed)

Order of combining P(Q) and S(Q) in Plugins seems to matter

Reported by: smk78 Owned by: awashington
Priority: critical Milestone: SasView 4.2.0
Component: SasView Keywords:
Cc: Work Package: SasView Bug Fixing

Description

First noticed by Sarah Rogers, verified by smk78.

This plugin will compile and appear to work:

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

model_info = load_model_info('ellipsoid+ellipsoid@hardsphere')
model_info.name = 'Take Two'
model_info.description = 'Ellipse + Ellipse * HS'
Model = make_model_from_info(model_info)

but this will not:

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

model_info = load_model_info('ellipsoid@hardsphere+ellipsoid')
model_info.name = 'Take Four'
model_info.description = 'Ellipse * HS + Ellipse'
Model = make_model_from_info(model_info)

Note the order of combining the P(Q)s and S(Q)s.

A .pyc is created in both cases but only the first plugin appears in the Fit Page plugin model category drop down. However both appear in the Advanced Plugin Model Editor drop down. If you select the second plugin in that and Run to compile the model you get this traceback:

Running model 'takefour.py'...

Error occurred:
Traceback (most recent call last):
  File "sas\sasgui\perspectives\calculator\pyconsole.pyc", line 53, in show_model_output
  File "sas\sasgui\perspectives\calculator\pyconsole.pyc", line 32, in check_model
  File "sasmodels\sasview_model.pyc", line 111, in load_custom_model
  File "sasmodels\custom\__init__.pyc", line 45, in load_custom_kernel_module
  File "sasmodels\custom\__init__.pyc", line 36, in load_module_from_path
  File "C:\Users\loq\.sasview\plugin_models\takefour.py", line 5, in <module>
  File "sasmodels\core.pyc", line 154, in load_model_info
  File "sasmodels\product.pyc", line 50, in make_product_info
TypeError: S needs radius_effective as first parameter

Now, interestingly, the first plugin (the one that does load in the Fit Page) only has the volfraction parameter from the hardsphere S(Q). The radius_effective parameter is missing.

So there are two questions:

1) Is the apparent dependence on the order of combining the P(Q)s and S(Q)s a design feature or a bug?

2) Why isn't the radius_effective parameter passed to the Fit Page?

But it gets more interesting. If you create this:

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

model_info = load_model_info('ellipsoid*hardsphere')
model_info.name = 'ellipstimesHS'
model_info.description = 'Ellipse * HS'
Model = make_model_from_info(model_info)

and then this:

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

model_info = load_model_info('ellipsoid+custom.ellipstimesHS')
model_info.name = 'Take Three'
model_info.description = 'Ellipse + Ellipse * HS'
Model = make_model_from_info(model_info)

It appears in the Fit Page model category drop down (like the first plugin above) but does contain the radius_effective parameter (unlike the first plugin above)!

Final question:

3) Which of these should be giving a model of [ (ellipsoid) + (ellipsoid * S(Q)) ]?

Attachments (1)

takethree.png (140.2 KB) - added by smk78 3 years ago.
Screenshot of different plugin examples

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by smk78

Screenshot of different plugin examples

comment:1 Changed 3 years ago by smk78

Forgot to add. This was using Jenkins Win 7 4.2.0 Build 803

comment:2 Changed 3 years ago by pkienzle

sasmodels.core.load_model_info is first splitting on '@' then splitting on '+' which gives the wrong precedence. Need to rearrange the function so that the '@' split happens after '+'.

comment:3 Changed 3 years ago by butler

  • Owner set to awashington
  • Status changed from new to assigned

comment:4 Changed 3 years ago by awashington

Pull requests 60 and 61 have both been offered as competing solutions to the problem.

Last edited 3 years ago by awashington (previous) (diff)

comment:5 Changed 3 years ago by butler

  • Resolution set to fixed
  • Status changed from assigned to closed

was not closed with merge of pull request so closing manually

comment:6 Changed 2 years ago by smk78

Have checked awashington's fix by pulling from master on 13/02/2018 10:30 and building locally. Looks good to me!

Note: See TracTickets for help on using tickets.