-
Notifications
You must be signed in to change notification settings - Fork 19
feat: add warning for extrapolation in morphsqueeze
#250
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 all commits
400851b
4f59c54
bdd29ae
0cbcee4
026984a
e165b41
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:** | ||
|
|
||
| * No news added: Add warning for extrapolation in morphsqueeze.py. | ||
|
|
||
| **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 |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| import subprocess | ||
|
|
||
| import numpy as np | ||
| import pytest | ||
| from numpy.polynomial import Polynomial | ||
|
|
@@ -85,3 +87,83 @@ def test_morphsqueeze(x_morph, x_target, squeeze_coeffs): | |
| assert np.allclose(x_morph_actual, x_morph_expected) | ||
| assert np.allclose(x_target_actual, x_target) | ||
| assert np.allclose(y_target_actual, y_target) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "squeeze_coeffs, wmsg_gen", | ||
| [ | ||
| # extrapolate below | ||
| ( | ||
| {"a0": 0.01}, | ||
| lambda x: ( | ||
| "Warning: points with grid value below " | ||
| f"{x[0]} will be extrapolated." | ||
| ), | ||
| ), | ||
| # extrapolate above | ||
| ( | ||
| {"a0": -0.01}, | ||
| lambda x: ( | ||
| "Warning: points with grid value above " | ||
| f"{x[1]} will be extrapolated." | ||
| ), | ||
| ), | ||
| # extrapolate below and above | ||
| ( | ||
| {"a0": 0.01, "a1": -0.002}, | ||
| lambda x: ( | ||
| "Warning: points with grid value below " | ||
| f"{x[0]} and above {x[1]} will be " | ||
| "extrapolated." | ||
| ), | ||
| ), | ||
| ], | ||
| ) | ||
| def test_morphsqueeze_extrapolate(user_filesystem, squeeze_coeffs, wmsg_gen): | ||
| x_morph = np.linspace(0, 10, 101) | ||
| y_morph = np.sin(x_morph) | ||
| x_target = x_morph | ||
| y_target = y_morph | ||
| morph = MorphSqueeze() | ||
| morph.squeeze = squeeze_coeffs | ||
| coeffs = [squeeze_coeffs[f"a{i}"] for i in range(len(squeeze_coeffs))] | ||
| squeeze_polynomial = Polynomial(coeffs) | ||
| x_squeezed = x_morph + squeeze_polynomial(x_morph) | ||
| with pytest.warns() as w: | ||
| x_morph_actual, y_morph_actual, x_target_actual, y_target_actual = ( | ||
| morph(x_morph, y_morph, x_target, y_target) | ||
| ) | ||
| assert len(w) == 1 | ||
| assert w[0].category is UserWarning | ||
| actual_wmsg = str(w[0].message) | ||
| expected_wmsg = wmsg_gen([min(x_squeezed), max(x_squeezed)]) | ||
| assert actual_wmsg == expected_wmsg | ||
|
|
||
| # CLI test | ||
| morph_file, target_file = create_morph_data_file( | ||
| user_filesystem / "cwd_dir", x_morph, y_morph, x_target, y_target | ||
| ) | ||
| run_cmd = ["diffpy.morph"] | ||
| run_cmd.extend(["--squeeze=" + ",".join(map(str, coeffs))]) | ||
| run_cmd.extend([str(morph_file), str(target_file)]) | ||
| run_cmd.append("-n") | ||
| result = subprocess.run(run_cmd, capture_output=True, text=True) | ||
| assert expected_wmsg in result.stderr | ||
|
|
||
|
|
||
| def create_morph_data_file( | ||
|
Collaborator
Author
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. Do we want to create a
Collaborator
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. That would be helpful! (see #233) |
||
| data_dir_path, x_morph, y_morph, x_target, y_target | ||
| ): | ||
| morph_file = data_dir_path / "morph_data" | ||
| morph_data_text = [ | ||
| str(x_morph[i]) + " " + str(y_morph[i]) for i in range(len(x_morph)) | ||
| ] | ||
| morph_data_text = "\n".join(morph_data_text) | ||
| morph_file.write_text(morph_data_text) | ||
| target_file = data_dir_path / "target_data" | ||
| target_data_text = [ | ||
| str(x_target[i]) + " " + str(y_target[i]) for i in range(len(x_target)) | ||
| ] | ||
| target_data_text = "\n".join(target_data_text) | ||
| target_file.write_text(target_data_text) | ||
| return morph_file, target_file | ||
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.
Do we want to remove these few lines of code? I can't find other references to the variables defined here.
Uh oh!
There was an error while loading. Please reload this page.
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.
These were variables we made before to store what you have now called begin_end_squeeze. You can delete it if you prefer using begin_end_squeeze.
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.
I find these variables provide a handy interface to test the extrapolated effect in
test_morphsqueeze, so I think we should keep them.