Add other normalization factor for s(q)

mattwthompson / scattering

Functions for analyzing molecular simulations according to scattering experiments

MIT License

9 stars 9 forks source link

Add other normalization factor for s(q) #47

Closed rmatsum836 closed 3 years ago

rmatsum836 commented 3 years ago

TRAVIS includes two normalization factors for the structure factor. This PR adds the normalization factor in equation 4 of DOI: 10.1002/cphc.201500473.

rmatsum836 commented 3 years ago

I believe I have the other normalization working. Still need to:

[x] Clean up code in scattering.py
[x] Add unit tests to cover both normalization cases

rmatsum836 commented 3 years ago

I contacted the lead developer of TRAVIS and identified a bug in how the structure factors are written out to CSV format. By manually fixing the bug, the Faber-Ziman and Ashcroft-Langreth structure factors in this package show much closer agreement with TRAVIS.

This bug fix should be included in the next version of TRAVIS.

rmatsum836 commented 3 years ago

In the input check on line 60, the error message says only 'fz' is supported, but the if statement accepts 'fz' and 'al'. On line 128 of the original, partial_sq has a +1 that is not present in the new version at line 125. I don't know if this is intended or not. Otherwise, it looks good to me.

Yes, I removed the +1 so that the output of these functions is identical to that of another analysis package.

rmatsum836 commented 3 years ago

Tests are passing locally, will merge once CI has passed.