Skip to content

tests for scale_to #211

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

Merged
merged 9 commits into from
Dec 14, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/scaleto.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* functionality to rescale diffraction objects, placing one on top of another at a specified point

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
48 changes: 26 additions & 22 deletions src/diffpy/utils/diffraction_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,40 +391,44 @@ def on_tth(self):
def on_d(self):
return [self.all_arrays[:, 3], self.all_arrays[:, 0]]

def scale_to(self, target_diff_object, xtype=None, xvalue=None):
def scale_to(self, target_diff_object, q=None, tth=None, d=None, offset=0):
"""
Return a new diffraction object which is the current object but recaled in y to the target
returns a new diffraction object which is the current object but rescaled in y to the target

The y-value in the target at the closest specified x-value will be used as the factor to scale to.
The entire array is scaled by this factor so that one object places on top of the other at that point.
If multiple values of `q`, `tth`, or `d` are provided, or none are provided, an error will be raised.

Parameters
----------
target_diff_object: DiffractionObject
the diffraction object you want to scale the current one on to
xtype: string, optional. Default is Q
the xtype, from {XQUANTITIES}, that you will specify a point from to scale to
xvalue: float. Default is the midpoint of the array
the y-value in the target at this x-value will be used as the factor to scale to.
The entire array is scaled be the factor that places on on top of the other at that point.
xvalue does not have to be in the x-array, the point closest to this point will be used for the scaling.
the diffraction object you want to scale the current one onto

q, tth, d : float, optional, must specify exactly one of them
the xvalue (in `q`, `tth`, or `d` space) to align the current and target objects
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The value of the x-array where you want the curves to line up vertically. Specify a value on one of the allowed grids, q, tth, or d), e.g., q=10."


offset : float, optional, default is 0
an offset to add to the scaled y-values

Returns
-------
the rescaled DiffractionObject as a new object

"""
scaled = deepcopy(self)
if xtype is None:
xtype = "q"
scaled = self.copy()
count = sum([q is not None, tth is not None, d is not None])
if count != 1:
raise ValueError(
"You must specify exactly one of 'q', 'tth', or 'd'. Please rerun specifying only one."
)

xtype = "q" if q is not None else "tth" if tth is not None else "d" if d is not None else "q"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we drop the last "else "q""? given our validation above?

data, target = self.on_xtype(xtype), target_diff_object.on_xtype(xtype)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split to two lines for greater readability


data = self.on_xtype(xtype)
target = target_diff_object.on_xtype(xtype)
if xvalue is None:
xvalue = data[0][0] + (data[0][-1] - data[0][0]) / 2.0
xvalue = q if xtype == "q" else tth if xtype == "tth" else d

xindex = (np.abs(data[0] - xvalue)).argmin()
ytarget = target[1][xindex]
yself = data[1][xindex]
scaled.on_tth[1] = data[1] * ytarget / yself
scaled.on_q[1] = data[1] * ytarget / yself
xindex_data = (np.abs(data[0] - xvalue)).argmin()
xindex_target = (np.abs(target[0] - xvalue)).argmin()
scaled._all_arrays[:, 0] = data[1] * target[1][xindex_target] / data[1][xindex_data] + offset
return scaled

def on_xtype(self, xtype):
Expand Down
136 changes: 136 additions & 0 deletions tests/test_diffraction_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,142 @@ def test_init_invalid_xtype():
DiffractionObject(xtype="invalid_type")


params_scale_to = [
# UC1: same x-array and y-array, check offset
(
[
np.array([10, 15, 25, 30, 60, 140]),
np.array([2, 3, 4, 5, 6, 7]),
"tth",
2 * np.pi,
np.array([10, 15, 25, 30, 60, 140]),
np.array([2, 3, 4, 5, 6, 7]),
"tth",
2 * np.pi,
None,
60,
None,
2.1,
],
["tth", np.array([4.1, 5.1, 6.1, 7.1, 8.1, 9.1])],
),
# UC2: same length x-arrays with exact x-value match
(
[
np.array([10, 15, 25, 30, 60, 140]),
np.array([10, 20, 25, 30, 60, 100]),
"tth",
2 * np.pi,
np.array([10, 20, 25, 30, 60, 140]),
np.array([2, 3, 4, 5, 6, 7]),
"tth",
2 * np.pi,
None,
60,
None,
0,
],
["tth", np.array([1, 2, 2.5, 3, 6, 10])],
),
# UC3: same length x-arrays with approximate x-value match
(
[
np.array([0.12, 0.24, 0.31, 0.4]),
np.array([10, 20, 40, 60]),
"q",
2 * np.pi,
np.array([0.14, 0.24, 0.31, 0.4]),
np.array([1, 3, 4, 5]),
"q",
2 * np.pi,
0.1,
None,
None,
0,
],
["q", np.array([1, 2, 4, 6])],
),
# UC4: different x-array lengths with approximate x-value match
(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test example for scaling DOs with different array lengths. Here I think it makes more sense to scale them on q=61 (for self) & q=62 (for target).

[
np.array([10, 25, 30.1, 40.2, 61, 120, 140]),
np.array([10, 20, 30, 40, 50, 60, 100]),
"tth",
2 * np.pi,
np.array([20, 25.5, 32, 45, 50, 62, 100, 125, 140]),
np.array([1.1, 2, 3, 3.5, 4, 5, 10, 12, 13]),
"tth",
2 * np.pi,
None,
60,
None,
0,
],
# scaling factor is calculated at index = 5 for self and index = 6 for target
["tth", np.array([1, 2, 3, 4, 5, 6, 10])],
),
]


@pytest.mark.parametrize("inputs, expected", params_scale_to)
def test_scale_to(inputs, expected):
orig_diff_object = DiffractionObject(xarray=inputs[0], yarray=inputs[1], xtype=inputs[2], wavelength=inputs[3])
target_diff_object = DiffractionObject(
xarray=inputs[4], yarray=inputs[5], xtype=inputs[6], wavelength=inputs[7]
)
scaled_diff_object = orig_diff_object.scale_to(
target_diff_object, q=inputs[8], tth=inputs[9], d=inputs[10], offset=inputs[11]
)
# Check the intensity data is same as expected
assert np.allclose(scaled_diff_object.on_xtype(expected[0])[1], expected[1])


params_scale_to_bad = [
# UC1: user did not specify anything
(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the bad test case for specifying nothing

np.array([0.1, 0.2, 0.3]),
np.array([1, 2, 3]),
"q",
2 * np.pi,
np.array([0.05, 0.1, 0.2, 0.3]),
np.array([5, 10, 20, 30]),
"q",
2 * np.pi,
None,
None,
None,
0,
),
# UC2: user specified more than one of q, tth, and d
(
np.array([10, 25, 30.1, 40.2, 61, 120, 140]),
np.array([10, 20, 30, 40, 50, 60, 100]),
"tth",
2 * np.pi,
np.array([20, 25.5, 32, 45, 50, 62, 100, 125, 140]),
np.array([1.1, 2, 3, 3.5, 4, 5, 10, 12, 13]),
"tth",
2 * np.pi,
None,
60,
10,
0,
),
]


@pytest.mark.parametrize("inputs", params_scale_to_bad)
def test_scale_to_bad(inputs):
orig_diff_object = DiffractionObject(xarray=inputs[0], yarray=inputs[1], xtype=inputs[2], wavelength=inputs[3])
target_diff_object = DiffractionObject(
xarray=inputs[4], yarray=inputs[5], xtype=inputs[6], wavelength=inputs[7]
)
with pytest.raises(
ValueError, match="You must specify exactly one of 'q', 'tth', or 'd'. Please rerun specifying only one."
):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a test for error message

orig_diff_object.scale_to(target_diff_object, q=inputs[8], tth=inputs[9], d=inputs[10], offset=inputs[11])
Copy link
Contributor

@bobleesj bobleesj Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having inputs up to ,etc inputs[10] does not appear scalable to me and I found this was very hard to read and maintain in diffpy.snmf which I had to refactor: https://github.com/diffpy/diffpy.snmf/pull/120/files#diff-1bd6af744434d75c63490430b955f577f60277dfe95e9ad716e3f808a2ed9d48L85-L87

Discussion here:
#225 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way to resolve this future nightware could be having reusable instances of DiffractionObject defined under conftest.py with specific UC cases. Then, we import these instances through the parameters in each test func. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I agree in this case, this would be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, to make it more readable we could also pass the inputs as a dict so it would read input["wavelenght"] instead of input[0]. The intent of the former is much clearer.



params_index = [
# UC1: exact match
([4 * np.pi, np.array([30.005, 60]), np.array([1, 2]), "tth", "tth", 30.005], [0]),
Expand Down
Loading