From 815d24322ccd18777fd931613298e531a59396af Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Fri, 15 Sep 2023 11:18:08 -0700 Subject: [PATCH 1/5] Default behavior of divmod() is different from MATLAB mod(), on which the original logic was based. The prior logic threw a ZeroDivisionError if printitn == 0. Instead, this fix avoids this error by testing that printitn > 0. Tests added for various values of printitn. --- pyttb/cp_als.py | 2 +- tests/test_cp_als.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pyttb/cp_als.py b/pyttb/cp_als.py index b1254235..cd8f2cc7 100644 --- a/pyttb/cp_als.py +++ b/pyttb/cp_als.py @@ -238,7 +238,7 @@ def cp_als( # noqa: PLR0912,PLR0913,PLR0915 else: flag = 1 - if (divmod(iteration, printitn)[1] == 0) or (printitn > 0 and flag == 0): + if (printitn > 0) and ((divmod(iteration, printitn)[1] == 0) or (flag == 0)): print(f" Iter {iteration}: f = {fit:e} f-delta = {fitchange:7.1e}") # Check for convergence diff --git a/tests/test_cp_als.py b/tests/test_cp_als.py index a591790c..ca9a6c34 100644 --- a/tests/test_cp_als.py +++ b/tests/test_cp_als.py @@ -206,3 +206,28 @@ def test_cp_als_sptensor_zeros(capsys): capsys.readouterr() assert pytest.approx(output3["fit"], 1) == 0 assert output3["normresidual"] == 0 + + +@pytest.mark.indevelopment +def test_cp_als_tensor_printitn(capsys, sample_tensor): + (data, T) = sample_tensor + + # default printitn + (M, Minit, output) = ttb.cp_als(T, 2, printitn=1) + capsys.readouterr() + assert pytest.approx(output["fit"]) == 1 + + # zero printitn + (M, Minit, output) = ttb.cp_als(T, 2, printitn=0) + capsys.readouterr() + assert pytest.approx(output["fit"]) == 1 + + # negative printitn + (M, Minit, output) = ttb.cp_als(T, 2, printitn=-1) + capsys.readouterr() + assert pytest.approx(output["fit"]) == 1 + + # float printitn + (M, Minit, output) = ttb.cp_als(T, 2, printitn=1.5) + capsys.readouterr() + assert pytest.approx(output["fit"]) == 1 From cef6ac0093e89a277b7206e34a06110504b64f3e Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Fri, 15 Sep 2023 16:51:26 -0700 Subject: [PATCH 2/5] Update tests/test_cp_als.py Co-authored-by: Nick <24689722+ntjohnson1@users.noreply.github.com> --- tests/test_cp_als.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cp_als.py b/tests/test_cp_als.py index ca9a6c34..7e5d7e91 100644 --- a/tests/test_cp_als.py +++ b/tests/test_cp_als.py @@ -210,7 +210,7 @@ def test_cp_als_sptensor_zeros(capsys): @pytest.mark.indevelopment def test_cp_als_tensor_printitn(capsys, sample_tensor): - (data, T) = sample_tensor + _, T = sample_tensor # default printitn (M, Minit, output) = ttb.cp_als(T, 2, printitn=1) From e16e6fa08159e56fa20dbaf4354127a9293f37aa Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Fri, 15 Sep 2023 16:51:44 -0700 Subject: [PATCH 3/5] Update tests/test_cp_als.py Co-authored-by: Nick <24689722+ntjohnson1@users.noreply.github.com> --- tests/test_cp_als.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cp_als.py b/tests/test_cp_als.py index 7e5d7e91..4a61b41e 100644 --- a/tests/test_cp_als.py +++ b/tests/test_cp_als.py @@ -213,7 +213,7 @@ def test_cp_als_tensor_printitn(capsys, sample_tensor): _, T = sample_tensor # default printitn - (M, Minit, output) = ttb.cp_als(T, 2, printitn=1) + _, _, output = ttb.cp_als(T, 2, printitn=1) capsys.readouterr() assert pytest.approx(output["fit"]) == 1 From b86810328c3d756ef34e87213e188f4a2d01c6e5 Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Fri, 15 Sep 2023 17:00:32 -0700 Subject: [PATCH 4/5] Applying Nick's suggesetions to remove mark, change maxiters to save CPU cycles. Also removing output since nothing is actually checked. --- tests/test_cp_als.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/test_cp_als.py b/tests/test_cp_als.py index 4a61b41e..8f27b09a 100644 --- a/tests/test_cp_als.py +++ b/tests/test_cp_als.py @@ -213,21 +213,17 @@ def test_cp_als_tensor_printitn(capsys, sample_tensor): _, T = sample_tensor # default printitn - _, _, output = ttb.cp_als(T, 2, printitn=1) + ttb.cp_als(T, 2, printitn=1, maxiters=2) capsys.readouterr() - assert pytest.approx(output["fit"]) == 1 - + # zero printitn - (M, Minit, output) = ttb.cp_als(T, 2, printitn=0) + ttb.cp_als(T, 2, printitn=0, maxiters=2) capsys.readouterr() - assert pytest.approx(output["fit"]) == 1 - + # negative printitn - (M, Minit, output) = ttb.cp_als(T, 2, printitn=-1) + ttb.cp_als(T, 2, printitn=-1, maxiters=2) capsys.readouterr() - assert pytest.approx(output["fit"]) == 1 # float printitn - (M, Minit, output) = ttb.cp_als(T, 2, printitn=1.5) + ttb.cp_als(T, 2, printitn=1.5, maxiters=2) capsys.readouterr() - assert pytest.approx(output["fit"]) == 1 From cb3d4917ec6c7a55106b23e27a8c9d201abc2751 Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Fri, 15 Sep 2023 17:00:57 -0700 Subject: [PATCH 5/5] Removes mark --- tests/test_cp_als.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_cp_als.py b/tests/test_cp_als.py index 8f27b09a..f351922a 100644 --- a/tests/test_cp_als.py +++ b/tests/test_cp_als.py @@ -208,7 +208,6 @@ def test_cp_als_sptensor_zeros(capsys): assert output3["normresidual"] == 0 -@pytest.mark.indevelopment def test_cp_als_tensor_printitn(capsys, sample_tensor): _, T = sample_tensor