Skip to content

Squeeze morph: Adding UCs tests #182

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.

Sign up for GitHub

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
sbillinge merged 9 commits into from
Apr 19, 2025
Merged

Squeeze morph: Adding UCs tests #182

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c4ed0e6
test: adding test and empty function
Luiskitsu Apr 10, 2025
c59b09e
test: adding test and empty function
Luiskitsu Apr 10, 2025
2320b82
test: adding different UCs tests while keeping function empty
Luiskitsu Apr 11, 2025
ead3248
test: writting better docstrings and comments
Luiskitsu Apr 11, 2025
924872e
test: writting better docstrings and comments
Luiskitsu Apr 11, 2025
3538e4d
test: modified docstrings
Luiskitsu Apr 13, 2025
234b387
func: adding squeeze functionallity and extrapolation indices
Luiskitsu Apr 15, 2025
4c6d993
func: not returning extrapolation indices
Luiskitsu Apr 16, 2025
fcee820
test: made some coefficients smaller to pass tolerance
Luiskitsu Apr 18, 2025
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
Prev Previous commit
Next Next commit
func: adding squeeze functionallity and extrapolation indices
  • Loading branch information
@Luiskitsu
Luiskitsu committed Apr 15, 2025
commit 234b387cb7b5993e90b5067845b7a2a4870e2ee1
26 changes: 21 additions & 5 deletions src/diffpy/morph/morphs/morphsqueeze.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import numpy as np
from numpy.polynomial import Polynomial
from scipy.interpolate import CubicSpline

from diffpy.morph.morphs.morph import LABEL_GR, LABEL_RA, Morph


Expand Down Expand Up @@ -28,10 +32,13 @@ def morph(self, x_morph, y_morph, x_target, y_target):

Returns
-------
A tuple (x_morph_out, y_morph_out, x_target_out, y_target_out)
where the target values remain the same and the morph data
is shifted according to the squeeze. The morphed data is
returned on the same grid as the unmorphed data.
A tuple (x_morph_out, y_morph_out, x_target_out, y_target_out,
min_index, max_index) where the target values remain the same and

@sbillinge sbillinge Apr 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we don't need to return min and max index. These can be just updated in place. Also, these names are not so intuitive to me. maybe something like extrap_index_low and extrap_index_high? Let's define these and set them to None when MorphSqueeze is instantiated.

the morph data is shifted according to the squeeze. The morphed
data is returned on the same grid as the unmorphed data.
The min_index and max_index are the last index before the
interpolated region and the first index after the interpolated
region, respectively. If there is no extrapolation it returns None.

Example
-------
Expand All @@ -54,4 +61,13 @@ def morph(self, x_morph, y_morph, x_target, y_target):
"""
Morph.morph(self, x_morph, y_morph, x_target, y_target)

return self.xyallout
squeeze_polynomial = Polynomial(self.squeeze)
x_squeezed = self.x_morph_in + squeeze_polynomial(self.x_morph_in)
self.y_morph_out = CubicSpline(x_squeezed, self.y_morph_in)(
self.x_morph_in
)
left_extrap = np.where(self.x_morph_in < x_squeezed[0])[0]
right_extrap = np.where(self.x_morph_in > x_squeezed[-1])[0]
min_index = left_extrap[-1] if left_extrap.size else None

@sbillinge sbillinge Apr 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

self.extrap_index_low (or left) and don't return it.

Describe this variable in the constructor where it is first defined.

max_index = right_extrap[0] if right_extrap.size else None
return self.xyallout + (min_index, max_index)

@sbillinge sbillinge Apr 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

apart from it being a bad idea in general, this breaks backwards compatibility.

34 changes: 26 additions & 8 deletions tests/test_morphsqueeze.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
squeeze_coeffs_list = [
# The order of coefficients is [a0, a1, a2, ..., an]
# Negative cubic squeeze coefficients
[-0.2, -0.01, -0.001, -0.001],
[-0.2, -0.01, -0.001, -0.0001],
# Positive cubic squeeze coefficients
[0.2, 0.01, 0.001, 0.001],
[0.2, 0.01, 0.001, 0.0001],
# Positive and negative cubic squeeze coefficients
[0.2, -0.01, 0.002, -0.001],
[0.2, -0.01, 0.002, -0.0001],
# Quadratic squeeze coefficients
[-0.2, 0.005, -0.007],
# Linear squeeze coefficients
[0.1, 0.3],
# 4th order squeeze coefficients
[0.2, -0.01, 0.001, -0.001, 0.0004],
# Zeros and non-zeros, the full polynomial is applied
[0, 0.03, 0, -0.001],
[0, 0.03, 0, -0.0001],
# Testing zeros, expect no squeezing
[0, 0, 0, 0, 0, 0],
]
Expand Down Expand Up @@ -51,10 +51,28 @@ def test_morphsqueeze(x_morph, x_target, squeeze_coeffs):
y_morph_expected = np.sin(x_morph)
morph = MorphSqueeze()
morph.squeeze = squeeze_coeffs
x_morph_actual, y_morph_actual, x_target_actual, y_target_actual = morph(
x_morph, y_morph, x_target, y_target
)
assert np.allclose(y_morph_actual, y_morph_expected)
(
x_morph_actual,
y_morph_actual,
x_target_actual,
y_target_actual,
low_extrap_idx,
high_extrap_idx,
) = morph(x_morph, y_morph, x_target, y_target)
if low_extrap_idx is None and high_extrap_idx is None:

@sbillinge sbillinge Apr 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this seems too complicated to me, and also does it test the extrapolation? How about something like

if low_extrap_idx is None: 
    low_extrap_idx = 0
elif high_extrap_idx is None:
    high_extrap_idx = -1
assert np.allclose(y_morph_actual[low_extrap_index:high_extrap_idx], y_morph_expected[low_extrap_index:high_extrap_idx], atol=1e-6)
assert np.allclose(y_morph_actual[:low_extrap_index-1], y_morph_expected[:low_extrap_index-1], atol=1e-5)
assert np.allclose(y_morph_actual[high_extrap_index+1:], y_morph_expected[high_extrap_index+1:], atol=1e-5)
assert morph.low_extrap_idx == expected_low_extrap_idx
assert morph.high_extrap_idx == expected_high_extrap_idx

assert np.allclose(y_morph_actual, y_morph_expected, atol=1e-6)
else:
interp_start = low_extrap_idx + 1 if low_extrap_idx is not None else 0
interp_end = (
high_extrap_idx
if high_extrap_idx is not None
else len(y_morph_actual)
)
assert np.allclose(
y_morph_actual[interp_start:interp_end],
y_morph_expected[interp_start:interp_end],
atol=1e-6,
)
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)