Fix using `.A` instead of `.toarray()` #61

michalk8 · 2024-08-05T22:13:19Z

No description provided.

msmdev

I'm not really getting the motivation/justification for your changes here. Could you please explain the reasoning behind them to me? Thank you!

msmdev · 2024-08-30T13:01:10Z

tests/test_gpcca.py

@@ -524,6 +525,7 @@ def test_gpcca_brandts_sparse_is_not_densified(self, P: np.ndarray, sd: np.ndarr
        with pytest.raises(ValueError, match=r"Sparse implementation is only available for `method='krylov'`."):
            GPCCA(csr_matrix(P), eta=sd, method="brandts").optimize(3)

+    @pytest.mark.skipif(Version(np.__version__) >= Version("2"), reason="Eigenvalue mismatch.")


So you skip this test if numpy version is >=2? Why is it ok for newer numpy versions to fail this test?

I've been trying to figure out why new numpy version causes this test to fail, but was unable to figure it out. I will try later do a bisect on the commits, see exactly why, but was thinking in the meantime we could just skip it.

I've reverted this, will leave this for a future PR to investigate.

tests/conftest.py

michalk8 · 2024-08-30T15:57:12Z

I'm not really getting the motivation/justification for your changes here. Could you please explain the reasoning behind them to me? Thank you!

Regarding the changes in the docs, sphinx was throwing some warning, so I corrected the template and conf.py - no warnings now!

msmdev · 2024-11-09T17:43:25Z

There are currently a lot of strange issues:

failing sphinx during lint: "TypeError: can only concatenate str (not "_StrPath") to str")
strange errors for py3.10-noslepc and py3.9-slepc: "ERROR: InvocationError for command /home/runner/work/pyGPCCA/pyGPCCA/.tox/py3.10-noslepc/bin/python -m pytest --cov --cov-append --cov-report=term-missing --cov-config=/home/runner/work/pyGPCCA/pyGPCCA/tox.ini --ignore docs/ -vv (exited with code 1)"
the tests py3.11-noslepc and py3.12-noslepc still fail because of the known issues with test_sort_real_schur() and new numpy versions
the macos-latest-test pretends to succeed but actually fails with "ERROR: Cannot locate the conda executable."

Honestly, I don't understand what is going on here...

@michalk8 maybe you have an idea?

michalk8 · 2024-11-18T23:52:47Z

Fixed:

failing sphinx during lint: "TypeError: can only concatenate str (not "_StrPath") to str")
strange errors for py3.10-noslepc and py3.9-slepc: "ERROR: InvocationError for command /home/runner/work/pyGPCCA/pyGPCCA/.tox/py3.10-noslepc/bin/python -m pytest --cov --cov-append --cov-report=term-missing --cov-config=/home/runner/work/pyGPCCA/pyGPCCA/tox.ini --ignore docs/ -vv (exited with code 1)"
the macos-latest-test pretends to succeed but actually fails with "ERROR: Cannot locate the conda executable."

As for the failing tests with numpy>=2.0, I unfortunately have no idea; will try to go over the changelog and see what might've broken the tests.

In the future, I plan refactor/modernize the installation and building of docs.

michalk8 · 2024-11-18T23:53:29Z

.github/workflows/ci.yml

                name: ${{ matrix.os }}-${{ matrix.python }}-${{ matrix.slepc }}
-                env_vars: OS,PYTHON
-                fail_ci_if_error: false
+                token: ${{ secrets.CODECOV_TOKEN }}


@msmdev could you please create and add this token to the repo secrets?

Seems that it was already there in the repo secrets...

codecov-commenter · 2024-11-18T23:54:19Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.21%. Comparing base (ec4b04e) to head (8df37dc).
Report is 16 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (ec4b04e) and HEAD (8df37dc). Click for more details.

HEAD has 60 uploads less than BASE

Flag BASE (ec4b04e) HEAD (8df37dc)

unittests 60 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
- Coverage   77.80%   70.21%   -7.59%     
==========================================
  Files           7        8       +1     
  Lines         973      997      +24     
  Branches      172      173       +1     
==========================================
- Hits          757      700      -57     
- Misses        135      219      +84     
+ Partials       81       78       -3

Flag	Coverage Δ
unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

msmdev · 2024-12-02T16:32:18Z

Interestingly, now we are getting a different picture regarding the failing test, which was hidden by the seemingly passing MacOS tests:
while for Liunx test_gpcca.py::TestGPCCAMatlabUnit::test_sort_real_schur[X] (for X=M_2, ..., M_5) is failing in all cases (slepc & noslepc), for MacOS test_gpcca.py::TestGPCCAMatlabUnit::test_sort_real_schur[X] isn't failing but instead the following tests fail:

test_gpcca.py::TestCustom::test_P_2_LM[brandts],
test_gpcca.py::TestCustom::test_split_warning_LM,
test_gpcca.py::TestCustom::test_split_raise_LM,
test_gpcca.py::TestCustom::test_P_2_LR[brandts],
test_gpcca.py::TestCustom::test_P_2_LR[brandts],
test_gpcca.py::TestCustom::test_split_raise_LR

msmdev · 2024-12-02T16:36:00Z

This difference is quite interesting. Maybe it is a result of differences in system architecture? Do you know, on what type of processors the MacOS tests are running: Intel (as Linux) or ARM?

msmdev · 2024-12-02T16:42:21Z

@michalk8, thank you very much for solving the issues related to outdated runners, dependencies and several other legacy issues!
I will merge the PR now to get those improvements upstream.
We should look into the failing tests in a new PR as you suggested.

Fix using .A instead of .toarray()

ad04fa6

michalk8 added the bug Something isn't working label Aug 5, 2024

michalk8 self-assigned this Aug 5, 2024

michalk8 added 6 commits August 6, 2024 00:16

Fix a typo

ceefb6e

Print matrices

f93f2b7

Try numpy.linalg.eigvals

1dcda18

Fix class template in the docs

6be6b9a

Try BLAS/LAPACK from conda

92b7d29

Skip some tests in numpy>=2

74a553b

michalk8 requested a review from msmdev August 5, 2024 23:28

msmdev reviewed Aug 30, 2024

View reviewed changes

michalk8 added 2 commits October 11, 2024 15:05

Remove test skip

16ac8f6

Update GA's versions

b5ff555

michalk8 added 15 commits November 18, 2024 23:23

Update pre-commits

791eaec

Fix pyproject.toml

eb854da

Switch to myst-nb

1bc7e0c

Use amsmath

9eee309

Remove enviroment.yaml

d3b3c45

Drop tox-conda

71db382

Try conda in CI

64f8157

Fix typo

f84e9a2

Remove tox from CI

a1c4393

Don't skip on mpi4py

07b7003

Install petsc/slepc

5a53631

Specify correct shell

034845b

Change env name

8475c27

Remove option

b98a9e6

Fix conda CI

7b95822

Pin numpy

38a5330

michalk8 force-pushed the fix/scipy-sparse branch from 9d7d858 to 38a5330 Compare November 18, 2024 23:36

michalk8 added 5 commits November 19, 2024 00:40

Update codecov action

52915c0

Undo pin

f280d59

Update coverage

5b5d0a4

Fix typo

60210a0

Add token

8df37dc

michalk8 commented Nov 18, 2024

View reviewed changes

msmdev merged commit 773c463 into main Dec 2, 2024
4 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix using `.A` instead of `.toarray()` #61

Fix using `.A` instead of `.toarray()` #61

michalk8 commented Aug 5, 2024

msmdev left a comment

msmdev Aug 30, 2024

michalk8 Aug 30, 2024

michalk8 Oct 11, 2024

michalk8 commented Aug 30, 2024

msmdev commented Nov 9, 2024

michalk8 commented Nov 18, 2024

michalk8 Nov 18, 2024

msmdev Dec 2, 2024

codecov-commenter commented Nov 18, 2024 •

edited

Loading

msmdev commented Dec 2, 2024 •

edited

Loading

msmdev commented Dec 2, 2024

msmdev commented Dec 2, 2024

Fix using .A instead of .toarray() #61

Fix using .A instead of .toarray() #61

Conversation

michalk8 commented Aug 5, 2024

msmdev left a comment

Choose a reason for hiding this comment

msmdev Aug 30, 2024

Choose a reason for hiding this comment

michalk8 Aug 30, 2024

Choose a reason for hiding this comment

michalk8 Oct 11, 2024

Choose a reason for hiding this comment

michalk8 commented Aug 30, 2024

msmdev commented Nov 9, 2024

michalk8 commented Nov 18, 2024

michalk8 Nov 18, 2024

Choose a reason for hiding this comment

msmdev Dec 2, 2024

Choose a reason for hiding this comment

codecov-commenter commented Nov 18, 2024 • edited Loading

Codecov Report

msmdev commented Dec 2, 2024 • edited Loading

msmdev commented Dec 2, 2024

msmdev commented Dec 2, 2024

Fix using `.A` instead of `.toarray()` #61

Fix using `.A` instead of `.toarray()` #61

codecov-commenter commented Nov 18, 2024 •

edited

Loading

msmdev commented Dec 2, 2024 •

edited

Loading