-
Notifications
You must be signed in to change notification settings - Fork 17
test: adding a test to unsqueeze squeezed data #180
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
Changes from 3 commits
9a53b24
ac388e9
ce863d3
ca69295
a1b6425
14d92cb
a48ce43
d42d234
f47e167
4933b4f
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:** | ||
|
||
* Squeeze test: given a squeezed signal, unsqueezing recovers the original | ||
|
||
**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 |
---|---|---|
@@ -0,0 +1,74 @@ | ||
""" | ||
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. we don't need docstrings in test functions, so please delete this, (but we do like comments on each of the tests that capture the intended behavior of each test. |
||
The squeeze morph is used to correct for small non-linear geometric distortions | ||
from detectors that are not effectively corrected during calibration, such as | ||
individual module misalignment or tilt. This squeezing is applied as: | ||
x_squeezed = x + squeeze_0 + squeeze_1 * x**2 + squeeze_2 * x**3 | ||
|
||
The squeeze distortions that we might encounter practically are going to be | ||
very small. Furthermore, large values for the squeezing parameters lead to | ||
missing values during interpolation. Therefore is important to use small | ||
squeezing values to avoid error or unphysical results. | ||
Safe bounds for the squeezing parameters are: | ||
squeeze_0 [-0.2, 0.2] | ||
squeeze_1 [-0.001, 0.001] | ||
squeeze_2 [-0.0001, 0.0001] | ||
Values outside these bounds should be used carefully. | ||
Note that these bounds are established for an x-axis that goes from 0 to 10. | ||
""" | ||
|
||
import matplotlib.pyplot as plt | ||
import numpy as np | ||
from scipy.interpolate import interp1d | ||
|
||
|
||
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. we are now going to test 3 or 4 different variants of the same thing (i.e., different representative numbers) so it is time to use @pytest.paramatrize decorator. Please see |
||
def test_morphsqueeze(): | ||
""" | ||
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. see above, no docstrings here. |
||
Test that we can unsqueeze squeezed data. | ||
The test inputs are an expected uniform target (e.g. synchrotron data) | ||
and a squeezed version of the target (e.g. XFEL data). The squeezed data | ||
is created by applying a nonlinear distortion to the uniform target. | ||
Both input data are in the same uniform x-axis grid. | ||
Then we unsqueeze the squeezed data by doing the inverse transformation | ||
using interpolatiion. | ||
Finally we check that the unsqueezed data matches the expected uniform | ||
target. | ||
""" | ||
|
||
# Uniform x-axis grid. This is the same x-axis for all data. | ||
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 comments in the test are not needed, just the comments summarizing the behavior above are useful. |
||
x = np.linspace(0, 10, 1000) | ||
# Expected uniform target | ||
y_expected = np.sin(x) | ||
|
||
# Apply squeeze parameters to uniform data to get the squeezed data | ||
# Include squeeze_0 for squeezes with offset | ||
squeeze_0 = 0.2 | ||
squeeze_1 = -0.001 | ||
squeeze_2 = -0.001 | ||
x_squeezed = x + squeeze_0 + squeeze_1 * x**2 + squeeze_2 * x**3 | ||
y_squeezed = np.sin(x_squeezed) | ||
|
||
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 don't see where the function is called, so we don't seem to be testing anything. I would expect to see something like.
or sthg like that (not sure I have the syntax right), then
I expect this to fail initially because the function is not written yet, but write an empty function with just a docstring and a signature and then import it. Then write the code to make the test pass. |
||
# Unsqueeze the data by interpolating back to uniform grid | ||
# y_unsqueezed = np.interp(x, x_squeezed, y_squeezed) | ||
y_unsqueezed = interp1d( | ||
x_squeezed, | ||
y_squeezed, | ||
kind="cubic", | ||
bounds_error=False, | ||
fill_value="extrapolate", | ||
)(x) | ||
y_actual = y_unsqueezed | ||
|
||
# Check that the unsqueezed (actual) data matches the expected data | ||
# Including tolerance error because I was having issues | ||
# with y_actual == y_expected. I think is because interpolation? | ||
assert np.allclose(y_actual, y_expected, atol=100) | ||
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. this tolerance seems high? 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 sorry, it should be like 1 due to the extrapolation error. I will change this. |
||
|
||
# Plot to verify what we are doing | ||
plt.figure(figsize=(7, 4)) | ||
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. We don't want plotting code in the tests. You may want to leave these but commented out with a comment above saying that this plotting code was used for the comments in the github PR and give the URL. |
||
plt.plot(x, y_expected, color="black", label="Expected uniform data") | ||
plt.plot(x, y_squeezed, "--", color="purple", label="Squeezed data") | ||
plt.plot(x, y_unsqueezed, "--", color="gold", label="Unsqueezed data") | ||
plt.xlabel("x") | ||
plt.ylabel("y") | ||
plt.legend() | ||
plt.show() |
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.
you don't need this. This "news" is user facing documentation that will become the changelog at the next release. It tells users how the code has changed since the last release. Se we want to say something like, unded Added, * Polynomial squeeze of x-axis of morphed data" or something even more descriptive.