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

lamellarPS in sasview, bugs in polydispersity loops (and Caille S(Q) ) (Trac #376) #510

Closed
RichardHeenan opened this issue Mar 30, 2019 · 4 comments
Assignees
Labels
Defect Bug or undesirable behaviour Minor Small job
Milestone

Comments

@RichardHeenan
Copy link
Contributor

RichardHeenan commented Mar 30, 2019

Whilst debugging equivalent lamellarCaille routine for sasmodels,

discovered the old one had two bugs in the polydispersity.

Later Paul K also found a separate issue with a summation in the

Caille S(Q) itself. I need to check the status of the fix for this

also for this and lamellarPS_HG.

The polydispersity bugs are illustrated here:

// Perform the computation, with all weight points
double sum = 0.0;
double norm = 0.0;

// Loop over short_edgeA weight points
for(int i=0; i< (int)weights_spacing.size(); i++) {
dp[1] = weights_spacing[i].value;
//for(int j=0; j< (int)weights_spacing.size(); j++) { BUG
for(int j=0; j< (int)weights_delta.size(); j++) {
//dp[2] = weights_delta[i].value; BUG
dp[2] = weights_delta[j].value;

  sum += weights_spacing[i].weight * weights_delta[j].weight * LamellarPS_kernel(dp, q);
  norm += weights_spacing[i].weight * weights_delta[j].weight;
}

}
return sum/norm + background();

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

{
    "status": "closed",
    "changetime": "2015-03-24T16:12:32",
    "_ts": "2015-03-24 16:12:32.735829+00:00",
    "description": "Whilst debugging equivalent lamellarCaille routine for sasmodels, [[BR]]\ndiscovered the old one had two bugs in the polydispersity. [[BR]]\nLater Paul K also found a separate issue with a summation in the [[BR]]\nCaille S(Q) itself. I need to check the status of the fix for this [[BR]]\nalso for this and lamellarPS_HG.\n\nThe polydispersity bugs are illustrated here:\n\n  // Perform the computation, with all weight points\n  double sum = 0.0;\n  double norm = 0.0;\n\n  // Loop over short_edgeA weight points\n  for(int i=0; i< (int)weights_spacing.size(); i++) {\n    dp[1] = weights_spacing[i].value;\n    //for(int j=0; j< (int)weights_spacing.size(); j++) {    BUG\n    for(int j=0; j< (int)weights_delta.size(); j++) {\n      //dp[2] = weights_delta[i].value;        BUG\n      dp[2] = weights_delta[j].value;\n\n      sum += weights_spacing[i].weight * weights_delta[j].weight * LamellarPS_kernel(dp, q);\n      norm += weights_spacing[i].weight * weights_delta[j].weight;\n    }\n  }\n  return sum/norm + background();\n",
    "reporter": "richardh",
    "cc": "",
    "resolution": "fixed",
    "workpackage": "SasView Bug Fixing",
    "time": "2015-03-09T16:34:17",
    "component": "SasView",
    "summary": "lamellarPS in sasview, bugs in polydispersity loops (and Caille S(Q) )",
    "priority": "minor",
    "keywords": "lamellarPS",
    "milestone": "SasView 3.1.0",
    "owner": "richardh",
    "type": "defect"
}
@RichardHeenan RichardHeenan added this to the SasView 3.1.0 milestone Mar 30, 2019
@RichardHeenan RichardHeenan self-assigned this Mar 30, 2019
@RichardHeenan RichardHeenan added Defect Bug or undesirable behaviour Incomplete Migration Minor Small job and removed Incomplete Migration labels Mar 30, 2019
@RichardHeenan
Copy link
Contributor Author

Trac update at 2015/03/09 16:47:36: richardh commented:

Have fixed the Caille s(Q) in sasmodels for lamellarCaille, as Paul K had only done lamellarCailleHG. RichardH

@RichardHeenan
Copy link
Contributor Author

Trac update at 2015/03/09 18:04:47:

  • richardh changed _comment0 from "Have now pushed the fixes for lamellarPS and lamellarPS_HG in sasview, can close the ticket." to "1425924465577707"
  • richardh commented:

Have now pushed the fixes for lamellarPS and lamellarPS_HG in sasview. Alas they will now fail the current unit tests, as the results of the routines have changed!

@RichardHeenan
Copy link
Contributor Author

Trac update at 2015/03/09 18:33:01: richardh commented:

Have made a quick fix to utest_other_models.py , have to go home now, will check more tomorrow.

@pkienzle
Copy link
Contributor

Trac update at 2015/03/24 16:12:32:

  • pkienzle changed resolution from "" to "fixed"
  • pkienzle changed status from "new" to "closed"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Bug or undesirable behaviour Minor Small job
Projects
None yet
Development

No branches or pull requests

2 participants