-
Notifications
You must be signed in to change notification settings - Fork 21
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
tests for scale_to #211
Changes from 7 commits
d65dce1
5d07b9a
8a3ebd5
3c5a426
409cd42
79161f1
1b7aa14
c22a95b
53413fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -391,40 +391,45 @@ 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, an error will be raised. | ||
If none are provided, the midpoint of the current object's `q`-array will be used. | ||
|
||
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, default is the midpoint of the current object's `q`-array | ||
the xvalue (in `q`, `tth`, or `d` space) to align the current and target objects | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, |
||
|
||
sbillinge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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() | ||
sbillinge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
count = sum([q is not None, tth is not None, d is not None]) | ||
if count > 1: | ||
raise ValueError("You can only specify 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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
xvalue = q if xtype == "q" else tth if xtype == "tth" else d | ||
if xvalue is None: | ||
xvalue = data[0][0] + (data[0][-1] - data[0][0]) / 2.0 | ||
xvalue = (data[0][0] + data[0][-1]) / 2.0 | ||
|
||
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 | ||
x_data, x_target = (np.abs(data[0] - xvalue)).argmin(), (np.abs(target[0] - xvalue)).argmin() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. compute different indices for the two diffraction objects There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's change the variable name to |
||
y_data, y_target = data[1][x_data], target[1][x_target] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this line makes things less readable. I would put the |
||
scaled._all_arrays[:, 0] = data[1] * y_target / y_data + offset | ||
return scaled | ||
|
||
def on_xtype(self, xtype): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -180,6 +180,144 @@ 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 | ||
( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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])], | ||
), | ||
# UC5: user did not specify anything, use the midpoint of the current object's q-array | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added a test 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, | ||
], | ||
["q", np.array([10, 20, 30])], | ||
), | ||
] | ||
|
||
|
||
@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 = [ | ||
( | ||
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 can only specify one of 'q', 'tth', or 'd'. Please rerun specifying only one." | ||
): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. having inputs up to ,etc Discussion here: There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I agree in this case, this would be helpful. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
|
||
params_index = [ | ||
# UC1: exact match | ||
([4 * np.pi, np.array([30.005, 60]), np.array([1, 2]), "tth", "tth", 30.005], [0]), | ||
|
Uh oh!
There was an error while loading. Please reload this page.