Doc Update: Polydispersity #1938
Conversation
|
I think some additions are now needed to polydispersity.rst in Sasmodels. But that will have to be a separate PR. |
There was a problem hiding this comment.
OMG!!! this rst is HUUUGE 😄 with a number of things that need updating by the looks of it from just a quick skim. It is hard to keep these uptodate sometimes. I am wondering if we need to think about breaking this up somehow into several sub pages, both for the benefit of the user and for maintenance?
That said the PD addition seems correct and complete so I approve the PR for merging
|
I note however that this has conflicting changes in fitting_help.rst blocking an actual merge. This will have to be resolved before we can merge it. |
| particular, pay attention to the Suggested Applications and Usage Notes therein. | ||
| Note that SasView defaults to Gaussian distributions, but these will not always | ||
| be the best choice. Also, the definition of *PD[ratio]* varies depending on | ||
| the chosen distribution! |
There was a problem hiding this comment.
PD for orientation is absolute; PD for volume parameters are relative to the current value. It doesn't depend on the chosen distribution, though the meaning of center and width does (first and second moments was a good analytic choice, but it sure was confusing for rectangular distributions).
There was a problem hiding this comment.
Thanks @pkienzle . Have changed it to read now:
|
|
||
| .. image:: pd_tab.png | ||
|
|
||
| For more information, see the descriptions of :ref:`polydispersityhelp` . In |
There was a problem hiding this comment.
Neither here nor the polydispersity docs mention that the distribution is a number density distribution, not a particle volume distribution. This is discussed in fitting_sq.rst and can be linked as PStheory.
There was a problem hiding this comment.
Actually, it does say in polydispersity.rst:
I've now also added a note to the same effect in fitting_help.rst.
There was a problem hiding this comment.
Probably better to phrase this as These are all implemented as number densities. The resulting scattering is the number average from the different-sized particles., but that would be a sasmodels PR.
Within the current text you may want to mention the ability to reparameterize the particle so that you can control polydispersity according to within-particle constraints (link to plugin.rst). For example, if the process which generates the particles constrains the aspect ratio but not the volume, or if it is a squishy particle that preserves volume but allows a range of aspect ratios for each volume. You may need to define your own distribution function to fully describe the model (link to polydispersity.rst user defined functions).
There was a problem hiding this comment.
I have fielded several questions from users about the details of the polydispersity calculation, which is why I suggest linking to PStheory in fitting_sq.rst.
This file also discusses the relationship between scale, volume fraction, effective radius, effective radius mode and structure factor mode, which is useful information for the discussion of interaction and mixture models. I think there was an attempt to link to it in on line 105 of this file:
Also see :ref:`Interaction_and_Mixture_Models` for further information.
but it is self-referential! Please change it to ref Interaction_Models, which is the label used in fitting_sq.rst.
There was a problem hiding this comment.
Still tweaking the "number-average" phrase. The distributions are implemented as simple distributions, but they are being used to compute the number average. Something like:
These distributions define the number density of particles. The resulting scattering is the number average over the distribution.
Probably not worth changing.
There was a problem hiding this comment.
Ok, several changes to polydispersity.rst:
-
Have added a link to the end of the opening sentence (that resolves to PStheory):
-
Have changed the 'number distribution' sentence to read:
-
In array distribution have changed the existing text after the example array to a Note (the existing text had grammatical & typo errors anyhow) and added a new sentence to the bottom:
There was a problem hiding this comment.
And in fitting_help.rst:
-
Have changed the link from :ref:
Interaction_and_Mixture_Modelsto :ref:Interaction_Modelsas requested. -
Have also added a link to :ref:
PStheory: from the Polydisperse Parameters sub-section:
-
Have revised the wording of the note regarding number density to echo that now in polydispersity.rst:
-
Added a new note:
-
And have added a section flagging that you can reparameterize models:
Breakage in yaml syntax coming from merge from main is interfering with CI. Try fixing it...
|
ready for testing on OSX |
|
Hmm, Jenkins is not happy about this: |
|
@wpotrzebowski That's odd. The link target was added to Sasmodels /doc/guide/plugin.rst in this commit SasView/sasmodels@3965bee which has been merged. Is it possible Jenkins was trying to build against an outdated Sasmodels? |
|
ready for testing on Windows |
|
I've just built this branch locally against the current Sasmodels master and the link resolves fine. I think it's a Jenkins issue. |
|
ready for testing on OSX |
This PR attempts to address some of the documentation deficiencies in SasView raised in #1839. There is a wider reporting issue that would require coding changes which is not addressed here.
Work so far: