core_shell_parallelepiped 1-D model is incorrect (Trac #786)

[pkienzle]

The 1D core shell parallelepiped model does not use the thickness or SLD of the shell in the C dimension in the calculation so it cannot possibly be correct. Changing these parameters in the model does not affect the scattering curve.

Also, it does not support length_b = 0 in the core dimension, even if there is a shell thickness in b.

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

{
    "status": "closed",
    "changetime": "2018-01-16T23:26:30",
    "_ts": "2018-01-16 23:26:30.517422+00:00",
    "description": "The 1D core shell parallelepiped model does not use the thickness or SLD of the shell in the C dimension in the calculation so it cannot possibly be correct.  Changing these parameters in the model does not affect the scattering curve.\n\nAlso, it does not support length_b = 0 in the core dimension, even if there is a shell thickness in b.",
    "reporter": "pkienzle",
    "cc": "",
    "resolution": "fixed",
    "workpackage": "SasView Bug Fixing",
    "time": "2016-10-15T16:15:45",
    "component": "SasView",
    "summary": "core_shell_parallelepiped 1-D model is incorrect",
    "priority": "critical",
    "keywords": "",
    "milestone": "SasView 4.2.0",
    "owner": "butler",
    "type": "defect"
}

[butlerpd]

Trac update at 2016/10/27 16:07:46:

• butler commented:

Have tested this more and following the inheritance tree it seems that the code in the NIST IGOR package has exactly the same behavior. This should be a straightforward fix?

• butler changed owner from "" to "butler"
• butler changed status from "new" to "accepted"

[butlerpd]

Trac update at 2017/03/20 04:58:17:

• butler changed attachment from "" to "DivyaSinghAppendixC.pdf"
• butler commented:

Appendix C of Divya Singh's Thesis

[butlerpd]

Trac update at 2017/03/20 04:58:46:

• butler changed _comment0 from:

OK ... so it turns out that this model was indeed originally written by Divya Singh as part of her thesis (appropriate appendix attached here) in 2008. It was reviewed and "approved" by Andrew Jackson for distribution in NIST IGOR macros in 2009. Looking at the code it seems several original bits that were wrong are commented out and replaced (presumably by Andrew)? more importantly, there is a comment at the beginning and next to several commented out sections that the current version does NOT support a shell in C.

Miguel converted the code to the sasmodels framework and noted a number of oddities and commented the code but felt he did not understand the math enough to change it. Looking at it more closely I believe that in fact there must be an error in the normalization to units of length_b at least, and possibly a few other places.

Problem: how to validate the shells. for shells = 0 you should recover the parallelipiped or rectangular prism but that does not test the shells. Also note that notes in the original code and the original docs point out that A<B<C and that if they are not in that order the answer will be incorrect even though the calculation will return a value. So checking symmetric behavior is not an option either. It may be that someone needs to verify the math in the attached then implement and someone else independently verify the math and the implementation?

to:

1489986012212630

• butler commented:

OK ... so it turns out that this model was indeed originally written by Divya Singh as part of her thesis (appropriate appendix attached here) in 2008. It was reviewed and "approved" by Andrew Jackson for distribution in NIST IGOR macros in 2009. Looking at the code it seems several original bits that were wrong are commented out and replaced (presumably by Andrew)? more importantly, there is a comment at the beginning and next to several commented out sections that the current version does NOT support a shell in C. This explains the observed behavior reported in this ticket.

Miguel converted the code to the sasmodels framework and noted a number of oddities and commented the code but felt he did not understand the math enough to change it. Looking at it more closely I believe that in fact there must be an error in the normalization to units of length_b at least, and possibly a few other places.

Problem: how to validate the shells. for shells = 0 you should recover the parallelipiped or rectangular prism but that does not test the shells. Also note that notes in the original code and the original docs point out that A<B<C and that if they are not in that order the answer will be incorrect even though the calculation will return a value. So checking symmetric behavior is not an option either. It may be that someone needs to verify the math in the attached then implement and someone else independently verify the math and the implementation?

[butlerpd]

Trac update at 2017/03/24 04:21:30: butler commented:

In changeset cb0dc22aeaff717fb8130fd48c3134fa935d7ceb:

#!CommitTicketReference repository="sasmodels" revision="cb0dc22aeaff717fb8130fd48c3134fa935d7ceb"
document known issue in cs parallelepiped and the fact that it is not
completely understood and under review.  ticken #892 can now be moved to
release 4.2

[butlerpd]

Trac update at 2017/03/24 04:25:47:

• butler commented:

We do not have time to do the comprehensive review necessary on this model for release 4.1 However this model has been available in the NIST IGOR macros since 2009. Any errors will be long standing. Rather than pull long available models we don't fully understand have added notes to documentation to warn user that the !SasView team has yet to fully understand the code and that the code is under review. With that in place (and possibly also as release notes) this ticket can now move to 4.2

• butler changed milestone from "SasView 4.1.0" to "SasView 4.2.0"
• butler changed priority from "major" to "critical"

[sasview-bot]

Trac update at 2017/10/29 11:38:50: dirk commented:

The code for the core-shell parallelepiped is rewritten and it should be in-line with the math from the thesis appendix, which looks OK. Changes are here: [https://github.com/SasView/sasmodels/commit/c69d6d6b7d1b4a8e1acc9c734f6ec3b26084cdd7]. Trac did not catch my commit message with "ticket #892" in it.

[pkienzle]

Trac update at 2017/11/01 19:03:24: pkienzle commented:

Still inconsistent with orientation average of the 2-D model, and furthermore code is inconsistent with the 1D integral used in hollow rectangular prism. A fix is implemented in PR #283 of sasmodels.