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

New Model Editor needs to properly deal with form_volume function (Trac #885) #1834

Open
ajj opened this issue Mar 30, 2019 · 8 comments
Open
Assignees

Comments

@ajj
Copy link
Member

ajj commented Mar 30, 2019

The 'New Model Editor' allows you to specify parameters which are polydisperse and which are then marked as 'volume' parameters in the resulting code.

However, because form_volume still returns 1.0 by default, the weighting of the polydispersity is probably wrong.

This GUI should require the user to define form_volume if polydisperse parameters are requested.

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

{
    "status": "assigned",
    "changetime": "2019-03-01T23:46:19",
    "_ts": "2019-03-01 23:46:19.435246+00:00",
    "description": "The 'New Model Editor' allows you to specify parameters which are polydisperse and which are then marked as 'volume' parameters in the resulting code.\n\nHowever, because form_volume still returns 1.0 by default, the weighting of the polydispersity is probably wrong.\n\nThis GUI should require the user to define form_volume if polydisperse parameters are requested.",
    "reporter": "ajj",
    "cc": "",
    "resolution": "",
    "workpackage": "SasModels Infrastructure",
    "time": "2017-03-16T19:50:17",
    "component": "SasView",
    "summary": "New Model Editor needs to properly deal with form_volume function",
    "priority": "critical",
    "keywords": "",
    "milestone": "SasView 4.3.0",
    "owner": "gonzalezm",
    "type": "defect"
}
@butlerpd
Copy link
Member

Trac update at 2018/02/13 03:03:07: butler changed priority from "major" to "critical"

@butlerpd
Copy link
Member

Trac update at 2018/02/13 15:12:10:

  • butler changed owner from "" to "gonzalezm"
  • butler changed status from "new" to "assigned"

@butlerpd
Copy link
Member

Trac update at 2018/06/12 08:27:56:

  • butler commented:

This code has now been reverted to not support any polydispersity so that at least an easy model will be created correctly for release 4.2. branch 885 remains to work on fixing the editor to use polydispersity correctly but the ticket is moving to 4.3

  • butler changed milestone from "SasView 4.2.0" to "SasView 4.3.0"

@butlerpd
Copy link
Member

Trac update at 2018/08/07 14:27:33: butler commented:

The problems were almost surely due to the problems documented in ticket #1183 which should be fixed in 4.2.0. This branch can then be revived and merged.

@butlerpd
Copy link
Member

Trac update at 2019/03/01 23:46:19: butler changed workpackage from "SasView Bug Fixing" to "SasModels Infrastructure"

@ricleal ricleal transferred this issue from SasView/sasview Apr 23, 2019
@butlerpd butlerpd transferred this issue from SasView/sasmodels May 2, 2021
@butlerpd butlerpd added this to the SasView 4.3.0 milestone May 2, 2021
@butlerpd
Copy link
Member

butlerpd commented May 2, 2021

This should not have been moved to sasmodels as it is a GUI thing. I'm also not sure it is something we should consider fixing and assuming that it doesn't apply to 5.x (Though we should check that before closing this completely.

@RichardHeenan
Copy link
Contributor

This should not have been moved to sasmodels as it is a GUI thing. I'm also not sure it is something we should consider fixing and assuming that it doesn't apply to 5.x (Though we should check that before closing this completely.

5.x still requires form_volume with all the polydisperse parameters passed through (regardless of whether they actually contribute to the volume), so I think the new model editor should still be restricted to monodisperse - leaving users to manually edit the code if they later want polydispersity.

@butlerpd
Copy link
Member

Having now worked through using this as a demonstration in training, I would argue we should leave as is with perhaps some additions later to make it even more user friendly and hard to mess up. The current editor does behave differently if you include polydisperse parameters:

  1. with no polydisperse parameters, the add model editor just generates def Iq(x,[parameter list]) along with all the basic template including a start at the documentation and commented out stuff at the end that can be uncommented for stuff such as vectorized=true, along with instructions.
  2. If a polydisperse parameter is included, that parameter is labelled as a volume parameter and the editor adds three new functions: def form_volume([volume parameters]); def ER(); and def VR(). form_volume returns zero while the other two return one. The user still has to edit form_volume to return the actual volume and REMOVE a 1/V term from their original equation in order to return the number average for each polydisperse parameter. However, if they don't, it is not really wrong - the polydisperse parameters returned will be z averages instead of the number average. This of course will be confusing to many people for the very reason that SasView returns the number average in the first place: most material scientist (other than maybe polymer scientists) are unaware of the concept of weight or z averages vs number averages. For that reason the 3rd point below would be very helpful for these people
  3. What would be nice to add a separate place for form_volume (with instructions on how to use) in the editor itself. Actually this could be by adding it to the model program window after return y Alternatively it could be a completely second programming entry box. In other words a second text entry box would appear in the editor window once a polydisperse parameter is defined. This might make it clearer for the user. It could also add as a default
volume = polyparam1 * polyparam2 * polyparam3

return volume

@lucas-wilkins lucas-wilkins removed this from the SasView 4.3.0 milestone Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants