-
Notifications
You must be signed in to change notification settings - Fork 53
Different SLDs for Diffraction Pattern #1341
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
base: main
Are you sure you want to change the base?
Conversation
if not _HAS_MPL: | ||
raise ImportError(msg_mpl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
if not _HAS_MPL: | ||
raise ImportError(msg_mpl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
_HAS_MPL = find_spec("matplotlib") is not None | ||
if _HAS_MPL: | ||
import matplotlib.cm | ||
import matplotlib.colors | ||
|
||
import freud.plot | ||
else: | ||
msg_mpl = "Plotting requires matplotlib." | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
@@ -13,7 +13,6 @@ | |||
""" | |||
|
|||
import logging | |||
from importlib.util import find_spec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
import freud.plot | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
if not _HAS_MPL: | ||
raise ImportError(msg_mpl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
if not _HAS_MPL: | ||
raise ImportError(msg_mpl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
|
||
if vmax is None: | ||
vmax = 0.7 * self.N_points | ||
|
||
import freud.plot | ||
|
||
return freud.plot.diffraction_plot( | ||
self.diffraction, self.k_values, self.N_points, ax, cmap, vmin, vmax | ||
) | ||
|
||
def _repr_png_(self): | ||
try: | ||
import freud.plot | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To appease the pre-commit and the linter you want to revert the changes on the conditional imports to the code that is current with the main branch.
weights ((:math:`N`,) :class:`numpy.ndarray`, optional): | ||
SLD weights for each point in the system. If provided, the | ||
diffraction pattern will be normalized such that the sum of the | ||
weights is equal to the number of points in the system, resulting in | ||
:math:`S(0) = N`. If not provided, all points are assumed to | ||
have a weight of 1. (Default value = :code:`None`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first mention of SLD. Give the full meaning of the acronym when you first mention it.
@@ -237,3 +237,21 @@ def test_noncubic_system(self, noncubic_box_params): | |||
dp = freud.diffraction.DiffractionPattern() | |||
with pytest.raises(ValueError): | |||
dp.compute((box, points)) | |||
|
|||
def test_weighted_diffraction(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add a test that test that S(0) is N, as you mention in the PR text?
I provided a little modification to the
DiffractionPattern
class, to account for differently strong scattering mixtures.Description
I added weights to the computation of the histogram, to account for the different SLDs. I normalized the weights in a way, that$S(q=0) = N$ still holds.
Motivation and Context
For mixed systems, it is possible, that different parts of the mixture have very different SLDs, resulting in a higher contribution of one part of the mixture to the signal. To have a scattering image, that is close to an experimental one, it is imperative to account for different SLDs.
How Has This Been Tested?
I added new tests, to ensure, that the changes do not change existing behavior, i.e.$S(q = 0) = N$ , otherwise it does not affect the code downstream.
Checklist: