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

Check background handling in P*S models #179

Open
RichardHeenan opened this issue Mar 30, 2019 · 14 comments
Open

Check background handling in P*S models #179

RichardHeenan opened this issue Mar 30, 2019 · 14 comments

Comments

@RichardHeenan
Copy link
Contributor

RichardHeenan commented Mar 30, 2019

[This ticket doesn't have any actions items. Ask Richard if we can close it.]

See general introduction to beta(Q) project in SasView/sasview#173

Need technical details here as to what to change in what order to develop product.py and other (?) routines to achieve immediate goals of simple sphere, cylinder & ellipsoid times simple S(Q) but keeping in mind some longer term goals for more options and improved usage of gpu (SasView/sasview#1142, SasView/sasview#526 )

Ideally any calculation of I(Q) should only have one "flat background" at the end, but we may have to stick with each P(Q) carrying a flat background around with it. 

Some detail discussion and use cases are required here.

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

{
    "status": "new",
    "changetime": "2018-10-30T22:41:50",
    "_ts": "2018-10-30 22:41:50.391781+00:00",
    "description": "See general introduction to beta(Q) project in SasView/sasmeta#8\n\nNeed technical details here as to what to change in what order to develop product.py and other (?) routines to achieve immediate goals of simple sphere, cylinder & ellipsoid times simple S(Q) but keeping in mind some longer term goals for more options and improved usage of gpu (#1091,\u00a0#392\u00a0)\n\nIdeally any calculation of I(Q) should only have one \"flat background\" at the end, but we may have to stick with each P(Q) carrying a flat background around with it.\u00a0\n\nSome detail discussion and use cases are required here.",
    "reporter": "richardh",
    "cc": "",
    "resolution": "",
    "workpackage": "Beta Approximation Project",
    "time": "2018-07-01T21:29:03",
    "component": "sasmodels",
    "summary": "middle level - sort out P(Q)S(Q) including beta(Q)",
    "priority": "blocker",
    "keywords": "",
    "milestone": "sasmodels Next Release +1",
    "owner": "richardh",
    "type": "enhancement"
}
@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/03 11:02:42: richardh commented:

Suspect that we should first check that we are happy with the way the sum & product model parameters are being named, and that flat backgrounds and scale factors are being appropriately treated.

@butlerpd
Copy link
Member

Trac update at 2018/07/04 17:51:34: butler commented:

Scale and background were moved out of the model definition so that they can be more easily controlled for various combinations. Indeed I believe background is now only added ONCE to any combined model or it is supposed to be. Scale depends on whether it is a product or sum model: sums should have a scale for each since that allows for setting the fraction of I(Q) that is due to each model being summed incoherently, while the product model only has one scale factor I believe since 2 would be redundant.

This has caused a few other issue however, in that not all parts of the code may yet be aware yet that background and scale are added from outside the model code itself?

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 12:15:05:

  • richardh changed attachment from "" to "PS_test3_5July2018.svs"
  • richardh commented:

P(Q)S(Q) default compared to plugin for ellipsoid*hard_sphere, local build of 4.2

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 12:23:13: richardh commented:

Attached project, run from my local master build of 4.2, Fitpage1 & Fitpage2 ought to give identical fits, for ellipsoid x hard_sphere, though similar they are not the same! Nor do I yet understand the scale factor from the plugin model on Fitpage2, I am still investigating, but others might like to try different versions of the code.

Note the data set was loaded twice, once to send to each fit page, else the P(Q) and S(Q) overplots don't update as noted in other tickets. (Also I first checked that the default model gave same results on Fitpage2 as Fitpage1 when set up the same way.)

In terms of user functionality, the prefixed A_ and B_ strings on parameter names in the sum/multi could be chosen by the user.  For our default P(Q).S(Q) they could be an empty string for P(Q), ie unmodified, and say S(Q)_ for the S(Q) parameters.

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 14:56:36: richardh commented:

Ahhh 4.1.2 sum/multi generates ellipsoidxhard_sphere with an extra "BackGround" at the end of the parameters, with parameter names prefixed by p1_ and p2_.

Presume 4.2 mentioned in previous comment is supposed to be better.

Ahh going back to 4.2, just loading my 4.2 plugin model of ellipsoidxhard_sphere, I no longer get separate P(Q) and S(Q) in theories part of data window, which were previously there and updating, after I had previously used default P(Q)S(Q) on same Fitpage.

Is sum/multi plugin generator supposed to spot an S(Q) being used in 4.2 and take appropriate action?

Investigations continue ...

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 15:06:12:

  • richardh changed attachment from "" to "PS_test5_from_4p2_5July2018.svs"
  • richardh commented:

project P(Q)S(Q) test 5, from 4.2 local build

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 15:13:11: richardh commented:

PS_test5 project, all from 4.2, you may need a plugin model for FitPage2, to multiply ellipsoidx hard_sphere. Two screenshots show the parameters and fits.

Params on FitPage2 are copied from FitPage1 default fit, plus computed value of ER, adjusting only scale and backgroun now gets very close to expected agreement with FitPage1. I am however so far mystified as to why scale parameter fits to same number as B_volfraction in the S(Q), might have expected it to be 1.0 ???

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 15:13:42:

  • richardh changed attachment from "" to "PS_test5_default_model.png"
  • richardh commented:

PS_test5 default P(Q)S(Q)

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 15:14:15:

  • richardh changed attachment from "" to "PS_test5_plugin_model.png"
  • richardh commented:

PS test5 Plugin model to compare

@RichardHeenan
Copy link
Contributor Author

Trac update at 2018/07/05 15:36:51: richardh commented:

There are some comments on the development of sum/multi in SasView/sasview#174 , with saasview 3.1.2, 4.1.2 and 4.2 alll being different, so we should focus efforts on moving 4.2 forwards for anything in the beta(Q) project.

@butlerpd
Copy link
Member

Trac update at 2018/07/05 23:46:48: butler commented:

I don't believe sum/multiply pays attention to P vs S. I think the assumption is that PS already existing there would be no reason to do that with sum/multiply.... of course for the case of P1 + (SP2) there is no other way .. and one cannot save a standard P*S at the moment.

Replying to [comment:4 richardh]:

Ahhh 4.1.2 sum/multi generates ellipsoidxhard_sphere with an extra "BackGround" at the end of the parameters, with parameter names prefixed by p1_ and p2_.

Presume 4.2 mentioned in previous comment is supposed to be better.

Ahh going back to 4.2, just loading my 4.2 plugin model of ellipsoidxhard_sphere, I no longer get separate P(Q) and S(Q) in theories part of data window, which were previously there and updating, after I had previously used default P(Q)S(Q) on same Fitpage.

Is sum/multi plugin generator supposed to spot an S(Q) being used in 4.2 and take appropriate action?

Investigations continue ...

@butlerpd
Copy link
Member

Trac update at 2018/07/05 23:49:08: butler commented:

Indeed - the 4.2 sum/multiply is a very different structure. Currently (4.2.0) we are supporting all 3 versions (3.1.2, 4.1.2 and 4.2) but release notes will say something about probably not supporting the earlier 2 by version 5.0.

Replying to [comment:6 richardh]:

There are some comments on the development of sum/multi in SasView/sasview#174 , with saasview 3.1.2, 4.1.2 and 4.2 alll being different, so we should focus efforts on moving 4.2 forwards for anything in the beta(Q) project.

@pkienzle
Copy link
Contributor

Trac update at 2018/10/30 22:41:50:

  • pkienzle changed component from "SasView" to "sasmodels"
  • pkienzle changed milestone from "SasView Next Release +1" to "sasmodels Next Release +1"

@pkienzle pkienzle changed the title middle level - sort out P(Q)S(Q) including beta(Q) (Trac #1118) Check background handling in P*S models Apr 1, 2019
@butlerpd
Copy link
Member

butlerpd commented Apr 7, 2019

Agree with Paul Kienzle -- I think all actions here are either already fixed or in other tickets so we should probably close this. Maybe wait for verification from @RichardHeenan?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants