dnn should be lattice spacing for paracrystalline models

[pkienzle]

[Copied from ticket comment:4:ticket:805]

The parameter to the model is labelled as nearest neighbour distance, but I believe it is lattice spacing.

In units of lattice spacing, a, the q positions of the peaks are well determined by:

q = 2*pi/a * sqrt(h^{2 + k}2 + l²⁾

where SC allows all integer hkl, BCC requires h+k+l even and FCC require hkl all even or hkl all odd. Also need peak multiplicity to predict the peak height, since [100] = [-100] = [010] = [0-10] = [001] = [00-1], etc.

I checked peak position and height for dnn=20 and d_factor=0.06 for sc, bcc and fcc to verify the calculation.

We should rename dnn to length_a as in the parallelepiped model, and rename d_factor to lattice_distortion. Lattice distortion is actually proportional, Delta a / a, but distortion_proportion is too much of a mouthful.

[pkienzle]

Renaming parameters requires too much work for the 4.2 release.

Could simply update the docs with the correct parameter description and leave any parameter renaming to 4.3.

[butler]

Actually Andrew started a branch on that but did not issue a PR … partly because he made the changes but did not remember where the magic "conversion" file was that allows for backward compatibility.

[pkienzle]

Replying to butler:

Actually Andrew started a branch on that but did not issue a PR … partly because he made the changes but did not remember where the magic "conversion" file was that allows for backward compatibility.

Conversions go in sasmodels/convert.py.

Long term, may want to have version numbers for each model, and conversion functions within them. This would even work for plugin models.

This wouldn't handle model renaming, so still need convert.py or something like it.

[butler]

Looking at convert.py and conversion_table.py the infrastructure seems designed for translation from old style to new style models? Not at all clear to me how to use it for cases such as this? Assuming it can be used for such, some simple instruction somewhere might help?

[butler]

Actually reading the docs and then the code, the problems look like they go much deeper. I think dnn was in fact intended to be nearest neighbor *NOT* lattice spacing. Indeed the volume correction (ratio of sphere volume to unit cell volume) converts dnn (the nearest neighbor distance) to lattice spacing. NOTE: this seems to be done correctly in the code but is in incorrect in the BCC docs (both docs give the equation for FCC. SC also seems correct but then lattice and nearest neighbor are the same in this case). On the other hand the first peak position seems to suggest that dnn is the lattice spacing and the volume correction should only affect the intensity, though the P(Q) and distortion should move the peak position around. I suspect that whoever originally coded this model used the two (lattice and nearest neighbor spacing) interchangeably.

As noted by Michael Weir in an email:

Any chance there is confusion between parameters as Matsuoka paper uses "a_n" for nearest neighbour distance when you might reasonably expect "a" to refer to a lattice constant?

I believe the models need a complete going over and checked against the original references before going in and making changes. This probably means we need to move this to 4.3. Question would be if we change the vol ratio equation in BCC docs. We should add to Known Issues.

[richardh]

See also earlier comments and numerical issues in #975 and #805

Ought perhaps to for now put some large comments in the user docs as to where there may be issues.

[smk78]

I was just logging in to say exactly that… the Known Issues is too obscure.

[pkienzle]

Replying to butler:

Actually reading the docs and then the code, the problems look like they go much deeper. I think dnn was in fact intended to be nearest neighbor *NOT* lattice spacing. Indeed the volume correction (ratio of sphere volume to unit cell volume) converts dnn (the nearest neighbor distance) to lattice spacing.

In ticket:805#comment:4 point (6), I only checked the peak positions correspond to a rather than dnn. I did not check the volume fraction scaling factor.

Agreed that the code for volume fraction seems to be wrong.

• SC has one sphere per cell and should be V(sphere)/a³ = V(r/a).
• BCC has two spheres per cell and should be 2 V(r/a).
• FCC has four spheres per cell and should be 4 V(r/a).

[pkienzle]

I updated the code in the ticket_1156 branch to use lattice spacing for computing volume fraction.

[pkienzle]

I extended the real space sampling program (explore/realspace.py in the ticket_1156 branch) to support sc, bcc and fcc lattices so I could try comparing directly to our models.

We will not be able to use this to cross check that we have the correct code for the paracrystalline models. In particular, we won't be able to use it to check that we have the volume normalization correct. It may be that I have the wrong code for locating the sphere centers in the lattice, or it may be that there are tricks for simulating an infinite lattice with a finite number of samples.

The following gets some peaks in the correct place (2D)

$PYTHONPATH=. python explore/realspace.py --type=sc --shuffle=0.06 --view=10,20,30 --spacing=2,2,2 --lattice=8,8,8 -d 2 -s 50000 sphere radius=20

but has large intensity at q=0 which doesn't appear in the models. Increasing the number of lattice points or the number of samples doesn't improve things much.

Similarly, the 1D case (-d 1) is merely suggestive of the correct form without matching the details very well.

[butler]

At this point all remaining tickets should either be blockers or critical. Critical would be those that do not stop the actual release but need to be dealt with soon after such as updating the marketplace. Tickets such as this are blockers in that if not fixed the docs need to have an appropriate warning in them for this release. Once that is fixed the ticket moves to 4.3 if it is not closed.

[butler]

Looking at the original references, I believe the problem is that they do in fact mean nearest neighbor and are incorrect. We need to really think about this one I think before making changes.

[smk78]

I have added a warning to the top of the doc strings for the bcc, fcc and sc paracrystal models that says:

Warning: This model and this model description are under review following concerns raised by 
SasView users. If you need to use this model, please email help@sasview.org for the latest situation.
The SasView Developers. September 2018.

I've also harmonised the formatting of the references across all three.

[butler]

Now that the warnings are added this can be moved to 4.3

[butler]

If we can figure out what the right answer is this should get fixed in 4.2.1 - it has nearly zero danger of causing unforeseen repercussions

[butler]

There is no bandwidth to address this in 4.2.1 so moving to 4.3