Skip to content
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

Remove cvxpy for weight minimization, fix test values and refactor tests codes #120

Merged
merged 14 commits into from
Oct 30, 2024
25 changes: 25 additions & 0 deletions news/cvxpy.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
**Added:**

* L-BFGS-B method in scipy for weight optimization in def get_weights
* Support for Python 3.13

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* cvxpy dependency for linear weight optimization in def get_weights
* Support for Python 3.10

**Fixed:**

* Absolute tolerance for updated weighted matrix from 0.5 to 1e-05 in test_subroutines.py

**Security:**

* <news item>
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ maintainers = [
description = "A python package implementing the stretched NMF algorithm."
keywords = ['diffpy', 'PDF']
readme = "README.rst"
requires-python = ">=3.10, <3.13"
requires-python = ">=3.11, <3.14"
classifiers = [
'Development Status :: 4 - Beta',
'Environment :: Console',
Expand Down
1 change: 0 additions & 1 deletion requirements/conda.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
numpy
scipy
cvxpy
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As intended

diffpy.utils
numdifftools
1 change: 0 additions & 1 deletion requirements/pip.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
numpy
scipy
cvxpy
diffpy.utils
numdifftools
21 changes: 10 additions & 11 deletions src/diffpy/snmf/optimizers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import cvxpy
import numpy as np
from scipy.optimize import minimize


def get_weights(stretched_component_gram_matrix, linear_coefficient, lower_bound, upper_bound):
Expand Down Expand Up @@ -36,20 +36,19 @@ def get_weights(stretched_component_gram_matrix, linear_coefficient, lower_bound
input set. Has length C

"""

stretched_component_gram_matrix = np.asarray(stretched_component_gram_matrix)
linear_coefficient = np.asarray(linear_coefficient)
upper_bound = np.asarray(upper_bound)
lower_bound = np.asarray(lower_bound)

problem_size = max(linear_coefficient.shape)
solution_variable = cvxpy.Variable(problem_size)

objective = cvxpy.Minimize(
linear_coefficient.T @ solution_variable
+ 0.5 * cvxpy.quad_form(solution_variable, stretched_component_gram_matrix)
)
constraints = [lower_bound <= solution_variable, solution_variable <= upper_bound]
# Set dynamic bounds based on the size of the linear coefficient
bounds = [(lower_bound, upper_bound) for _ in range(len(linear_coefficient))]
initial_guess = np.zeros_like(linear_coefficient)

cvxpy.Problem(objective, constraints).solve()
# Find optimal weights of linear coefficients
def obj_func(y):
return linear_coefficient.T @ y + 0.5 * y.T @ stretched_component_gram_matrix @ y

return solution_variable.value
result = minimize(obj_func, initial_guess, method="L-BFGS-B", bounds=bounds)
return result.x
3 changes: 1 addition & 2 deletions src/diffpy/snmf/polynomials.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import numpy as np


def rooth(linear_coefficient, constant_term):
def compute_root(linear_coefficient, constant_term):
"""
Returns the largest real root of x^3+(linear_coefficient) * x + constant_term. If there are no real roots
return 0.
Expand All @@ -19,7 +19,6 @@ def rooth(linear_coefficient, constant_term):
The largest real root of x^3+(linear_coefficient) * x + constant_term if roots are real, else
return 0 array


"""
linear_coefficient = np.asarray(linear_coefficient)
constant_term = np.asarray(constant_term)
Expand Down
181 changes: 101 additions & 80 deletions tests/test_containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,59 @@

from diffpy.snmf.containers import ComponentSignal

tas = [
(
[np.arange(10), 3, 0, [6.55, 0.357, 8.49, 9.33, 6.78, 7.57, 7.43, 3.92, 6.55, 1.71], 0.25],
[
[6.55, 6.78, 6.55, 0, 0, 0, 0, 0, 0, 0],
[0, 14.07893122, 35.36478086, 0, 0, 0, 0, 0, 0, 0],
[0, -19.92049156, 11.6931482, 0, 0, 0, 0, 0, 0, 0],
],
),
(
[np.arange(5), 10, 0, [-11.47, -10.688, -8.095, -29.44, 14.38], 1.25],
[
[-11.47, -10.8444, -9.1322, -16.633, -20.6760],
[0, -0.50048, -3.31904, 40.9824, -112.1792],
[0, 0.800768, 5.310464, -65.57184, 179.48672],
],
),
(
[np.arange(5), 2, 0, [-11.47, -10.688, -8.095, -29.44, 14.38], 0.88],
[
[-11.47, -10.3344, -13.9164, -11.5136, 0],
[0, -3.3484, 55.1265, -169.7572, 0],
[0, 7.609997, -125.2876, 385.81189, 0],
],
),
(
[np.arange(10), 1, 2, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], 0.88],
[
[1, 2.1364, 3.2727, 4.4091, 5.5455, 6.6818, 7.8182, 8.9545, 0, 0],
[0, -1.29, -2.58, -3.87, -5.165, -6.45, -7.74, -9.039, 0, 0],
[0, 2.93, 5.869, 8.084, 11.739, 14.674, 17.608, 20.5437, 0, 0],
],
),
(
[

@pytest.mark.parametrize(
"grid, number_of_signals, id_number, iq, stretching_factor, expected",
[
(
np.arange(10),
3,
0,
[6.55, 0.357, 8.49, 9.33, 6.78, 7.57, 7.43, 3.92, 6.55, 1.71],
0.25,
[
[6.55, 6.78, 6.55, 0, 0, 0, 0, 0, 0, 0],
[0, 14.07893122, 35.36478086, 0, 0, 0, 0, 0, 0, 0],
[0, -19.92049156, 11.6931482, 0, 0, 0, 0, 0, 0, 0],
],
),
(
np.arange(5),
10,
0,
[-11.47, -10.688, -8.095, -29.44, 14.38],
1.25,
[
[-11.47, -10.8444, -9.1322, -16.633, -20.6760],
[0, -0.50048, -3.31904, 40.9824, -112.1792],
[0, 0.800768, 5.310464, -65.57184, 179.48672],
],
),
(
np.arange(5),
2,
0,
[-11.47, -10.688, -8.095, -29.44, 14.38],
0.88,
[
[-11.47, -10.3344, -13.9164, -11.5136, 0],
[0, -3.3484, 55.1265, -169.7572, 0],
[0, 7.609997, -125.2876, 385.81189, 0],
],
),
(
np.arange(10),
1,
2,
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10],
0.88,
[
[1, 2.1364, 3.2727, 4.4091, 5.5455, 6.6818, 7.8182, 8.9545, 0, 0],
[0, -1.29, -2.58, -3.87, -5.165, -6.45, -7.74, -9.039, 0, 0],
[0, 2.93, 5.869, 8.084, 11.739, 14.674, 17.608, 20.5437, 0, 0],
],
),
(
np.arange(14),
100,
3,
Expand All @@ -58,53 +76,56 @@
2.7960,
],
0.55,
],
[
[-2.9384, -1.9769, 0.9121, 0.6314, 0.8622, -2.4239, -0.2302, 1.9281, 0, 0, 0, 0, 0, 0],
[0, 2.07933, 38.632, 18.3748, 43.07305, -61.557, 26.005, -73.637, 0, 0, 0, 0, 0, 0],
[0, -7.56, -140.480, -66.81, -156.6293, 223.84, -94.564, 267.7734, 0, 0, 0, 0, 0, 0],
],
),
(
[np.arange(11), 20, 4, [0, 0.25, 0.5, 0.75, 1, 1.25, 1.5, 1.75, 2, 2.25, 2.5], 0.987],
[
[0, 0.2533, 0.5066, 0.7599, 1.0132, 1.2665, 1.5198, 1.7730, 2.0263, 2.2796, 0],
[0, -0.2566, -0.5132, -0.7699, -1.0265, -1.2831, -1.5398, -1.7964, -2.0530, -2.3097, 0],
[0, 0.5200, 1.0400, 1.56005, 2.08007, 2.6000, 3.1201, 3.6401, 4.1601, 4.6801, 0],
],
),
(
[np.arange(9), 15, 3, [-1, -2, -3, -4, -5, -6, -7, -8, -9], -0.4],
[[-1, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0]],
),
]


@pytest.mark.parametrize("tas", tas)
def test_apply_stretch(tas):
component = ComponentSignal(tas[0][0], tas[0][1], tas[0][2])
component.iq = tas[0][3]
component.stretching_factors[0] = tas[0][4]
[
[-2.9384, -1.9769, 0.9121, 0.6314, 0.8622, -2.4239, -0.2302, 1.9281, 0, 0, 0, 0, 0, 0],
[0, 2.07933, 38.632, 18.3748, 43.07305, -61.557, 26.005, -73.637, 0, 0, 0, 0, 0, 0],
[0, -7.56, -140.480, -66.81, -156.6293, 223.84, -94.564, 267.7734, 0, 0, 0, 0, 0, 0],
],
),
(
np.arange(11),
20,
4,
[0, 0.25, 0.5, 0.75, 1, 1.25, 1.5, 1.75, 2, 2.25, 2.5],
0.987,
[
[0, 0.2533, 0.5066, 0.7599, 1.0132, 1.2665, 1.5198, 1.7730, 2.0263, 2.2796, 0],
[0, -0.2566, -0.5132, -0.7699, -1.0265, -1.2831, -1.5398, -1.7964, -2.0530, -2.3097, 0],
[0, 0.5200, 1.0400, 1.56005, 2.08007, 2.6000, 3.1201, 3.6401, 4.1601, 4.6801, 0],
],
),
(
np.arange(9),
15,
3,
[-1, -2, -3, -4, -5, -6, -7, -8, -9],
-0.4,
[[-1, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0]],
),
],
)
def test_apply_stretch(grid, number_of_signals, id_number, iq, stretching_factor, expected):
component = ComponentSignal(grid, number_of_signals, id_number)
component.iq = iq
component.stretching_factors[0] = stretching_factor
actual = component.apply_stretch(0)
expected = tas[1]
np.testing.assert_allclose(actual, expected, rtol=1e-01)


taw = [
([np.arange(5), 2, 0, [0, 1, 2, 3, 4], 0.5], [0, 0.5, 1, 1.5, 2]),
([np.arange(5), 20, 2, [0, -1, -2, -3, -4], 0.25], [0, -0.25, -0.5, -0.75, -1]),
([np.arange(40), 200, 4, np.arange(0, 10, 0.25), 0.3], np.arange(0, 10, 0.25) * 0.3),
([np.arange(1), 10, 2, [10.5, 11.5, -10.5], 0], [0, 0, 0]),
([[-12, -10, -15], 5, 2, [-0.5, -1, -1.2], 0.9], [-0.45, -0.9, -1.08]),
([[-12, -10, -15], 5, 2, [0, 0, 0], 0.9], [0, 0, 0]),
]


@pytest.mark.parametrize("taw", taw)
def test_apply_weight(taw):
component = ComponentSignal(taw[0][0], taw[0][1], taw[0][2])
component.iq = np.array(taw[0][3])
component.weights[0] = taw[0][4]
@pytest.mark.parametrize(
"grid, number_of_signals, id_number, iq, weight, expected",
[
(np.arange(5), 2, 0, [0, 1, 2, 3, 4], 0.5, [0, 0.5, 1, 1.5, 2]),
(np.arange(5), 20, 2, [0, -1, -2, -3, -4], 0.25, [0, -0.25, -0.5, -0.75, -1]),
(np.arange(40), 200, 4, np.arange(0, 10, 0.25), 0.3, np.arange(0, 10, 0.25) * 0.3),
(np.arange(1), 10, 2, [10.5, 11.5, -10.5], 0, [0, 0, 0]),
([-12, -10, -15], 5, 2, [-0.5, -1, -1.2], 0.9, [-0.45, -0.9, -1.08]),
([-12, -10, -15], 5, 2, [0, 0, 0], 0.9, [0, 0, 0]),
],
)
def test_apply_weight(grid, number_of_signals, id_number, iq, weight, expected):
component = ComponentSignal(grid, number_of_signals, id_number)
component.iq = np.array(iq)
component.weights[0] = weight
actual = component.apply_weight(0)
expected = taw[1]
np.testing.assert_allclose(actual, expected, rtol=1e-01)
np.testing.assert_allclose(actual, expected, rtol=1e-04)
28 changes: 14 additions & 14 deletions tests/test_factorizers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@

from diffpy.snmf.factorizers import lsqnonneg

tl = [
([[[1, 0], [1, 0], [0, 1]], [2, 1, 1]], [1.5, 1.0]),
([[[2, 3], [1, 2], [0, 0]], [7, 7, 2]], [0, 2.6923]),
([[[3, 2, 4, 1]], [3.2]], [0, 0, 0.8, 0]),
([[[-0.4, 0], [0, 0], [-9, -18]], [-2, -3, -4.9]], [0.5532, 0]),
([[[-0.1, -0.2], [-0.8, -0.9]], [0, 0]], [0, 0]),
([[[0, 0], [0, 0]], [10, 10]], [0, 0]),
([[[2], [1], [-4], [-0.3]], [6, 4, 0.33, -5]], 0.767188240872451),
]


@pytest.mark.parametrize("tl", tl)
def test_lsqnonneg(tl):
actual = lsqnonneg(tl[0][0], tl[0][1])
expected = tl[1]
@pytest.mark.parametrize(
"stretched_component_matrix, target_signal, expected",
[
([[1, 0], [1, 0], [0, 1]], [2, 1, 1], [1.5, 1.0]),
([[2, 3], [1, 2], [0, 0]], [7, 7, 2], [0, 2.6923]),
([[3, 2, 4, 1]], [3.2], [0, 0, 0.8, 0]),
([[-0.4, 0], [0, 0], [-9, -18]], [-2, -3, -4.9], [0.5532, 0]),
([[-0.1, -0.2], [-0.8, -0.9]], [0, 0], [0, 0]),
([[0, 0], [0, 0]], [10, 10], [0, 0]),
([[2], [1], [-4], [-0.3]], [6, 4, 0.33, -5], 0.767188240872451),
],
)
def test_lsqnonneg(stretched_component_matrix, target_signal, expected):
actual = lsqnonneg(stretched_component_matrix, target_signal)
np.testing.assert_array_almost_equal(actual, expected, decimal=4)
28 changes: 14 additions & 14 deletions tests/test_optimizers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@

from diffpy.snmf.optimizers import get_weights

tm = [
([[[1, 0], [0, 1]], [1, 1], [0, 0], [1, 1]], [0, 0]),
([[[1, 0], [0, 1]], [1, 1], -1, 1], [-1, -1]),
([[[1.75, 0], [0, 1.5]], [1, 1.2], -1, 1], [-0.571428571428571, -0.8]),
([[[0.75, 0.2], [0.2, 0.75]], [-0.1, -0.2], -1, 1], [0.066985645933014, 0.248803827751196]),
([[[2, -1, 0], [-1, 2, -1], [0, -1, 2]], [1, 1, 1], -10, 12], [-1.5, -2, -1.5]),
([[[2, -1, 0], [-1, 2, -1], [0, -1, 2]], [1, -1, -1], -10, 12], [0, 1, 1]),
([[[4, 0, 0, 0], [0, 3, 0, 0], [0, 0, 2, 0], [0, 0, 0, 1]], [-2, -3, -4, -1], 0, 1000], [0.5, 1, 2, 1]),
]


@pytest.mark.parametrize("tm", tm)
def test_get_weights(tm):
expected = tm[1]
actual = get_weights(tm[0][0], tm[0][1], tm[0][2], tm[0][3])
@pytest.mark.parametrize(
"stretched_component_gram_matrix, linear_coefficient, lower_bound, upper_bound, expected",
[
([[1, 0], [0, 1]], [1, 1], 0, 0, [0, 0]),
([[1, 0], [0, 1]], [1, 1], -1, 1, [-1, -1]),
([[1.75, 0], [0, 1.5]], [1, 1.2], -1, 1, [-0.571428571428571, -0.8]),
([[0.75, 0.2], [0.2, 0.75]], [-0.1, -0.2], -1, 1, [0.066985645933014, 0.248803827751196]),
([[2, -1, 0], [-1, 2, -1], [0, -1, 2]], [1, 1, 1], -10, 12, [-1.5, -2, -1.5]),
([[2, -1, 0], [-1, 2, -1], [0, -1, 2]], [1, -1, -1], -10, 12, [0, 1, 1]),
([[4, 0, 0, 0], [0, 3, 0, 0], [0, 0, 2, 0], [0, 0, 0, 1]], [-2, -3, -4, -1], 0, 1000, [0.5, 1, 2, 1]),
],
)
def test_get_weights(stretched_component_gram_matrix, linear_coefficient, lower_bound, upper_bound, expected):
actual = get_weights(stretched_component_gram_matrix, linear_coefficient, lower_bound, upper_bound)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pass variables instead of tm[0][0], etc. which is much harder to read for future generations.

assert actual == pytest.approx(expected, rel=1e-4, abs=1e-6)
Loading