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.

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

Jump to bottom

BUG: sparse matrix creation in 1.13 with indices not summing entries any more #20670

Closed
m-reuter opened this issue May 8, 2024 · 2 comments · Fixed by #20687
Closed

BUG: sparse matrix creation in 1.13 with indices not summing entries any more #20670

m-reuter opened this issue May 8, 2024 · 2 comments · Fixed by #20687
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.sparse
Milestone
1.13.1

Comments

@m-reuter
Copy link

m-reuter commented May 8, 2024

Describe your issue.

The behaviour of sparse matrix (CSC, CSR) creation has changed in 1.13 breaking our code. Multiple data entries with the same indices are not summed up any longer but produce duplicate entries. I did not find this in the release notes. Maybe it is wanted this way, but the documentation explicitly states that conversion from COO to CSC sums up duplicate entries, so why drop this feature during creation?

Reproducing Code Example

from scipy import sparse
import numpy as np

dat = np.ones(3)
i = np.array((0,1,1))
j = np.array((1,0,0))

mat = sparse.csc_matrix((dat, (i, j)), shape=(2, 2))

# produces in 1.12:
print(mat)
  (1, 0)	2.0
  (0, 1)	1.0

# but in 1.13:
print(mat)
  (1, 0)	1.0
  (1, 0)	1.0
  (0, 1)	1.0

# converting it eg from COO to CSC will sum up the entries, but why not during creation?
mat2 = mat.tocoo()
>>> print(mat2)
  (1, 0)	1.0
  (1, 0)	1.0
  (0, 1)	1.0
>>> mat3 = mat2.tocsc()
>>> print(mat3)
  (1, 0)	2.0
  (0, 1)	1.0

Error message

No error message, just different sparse matrices at creation. I believe this is a bug, as the conversion, e.g. from COO format actually sums the entries as described here:
https://docs.scipy.org/doc/scipy/reference/sparse.html#example-2
which is important for FEM matrices or adjacency matrices of a mesh.

SciPy/NumPy/Python version and system information

1.13.0 1.26.4 sys.version_info(major=3, minor=10, micro=11, releaselevel='final', serial=0)
{
  "Compilers": {
    "c": {
      "name": "clang",
      "linker": "ld64",
      "version": "15.0.0",
      "commands": "cc"
    },
    "cython": {
      "name": "cython",
      "linker": "cython",
      "version": "3.0.10",
      "commands": "cython"
    },
    "c++": {
      "name": "clang",
      "linker": "ld64",
      "version": "15.0.0",
      "commands": "c++"
    },
    "fortran": {
      "name": "gcc",
      "linker": "ld64",
      "version": "13.2.0",
      "commands": "gfortran"
    },
    "pythran": {
      "version": "0.15.0",
      "include directory": "../../../../../../private/var/folders/l_/53d72k7j4zl9kj8fzncsd8hw0000gn/T/pip-build-env-b8vbzmu2/overlay/lib/python3.10/site-packages/pythran"
    }
  },
  "Machine Information": {
    "host": {
      "cpu": "aarch64",
      "family": "aarch64",
      "endian": "little",
      "system": "darwin"
    },
    "build": {
      "cpu": "aarch64",
      "family": "aarch64",
      "endian": "little",
      "system": "darwin"
    },
    "cross-compiled": false
  },
  "Build Dependencies": {
    "blas": {
      "name": "openblas",
      "found": true,
      "version": "0.3.26.dev",
      "detection method": "pkgconfig",
      "include directory": "/opt/arm64-builds/include",
      "lib directory": "/opt/arm64-builds/lib",
      "openblas configuration": "USE_64BITINT=0 DYNAMIC_ARCH=1 DYNAMIC_OLDER= NO_CBLAS= NO_LAPACK= NO_LAPACKE= NO_AFFINITY=1 USE_OPENMP= SANDYBRIDGE MAX_THREADS=64",
      "pc file directory": "/opt/arm64-builds/lib/pkgconfig"
    },
    "lapack": {
      "name": "openblas",
      "found": true,
      "version": "0.3.26.dev",
      "detection method": "pkgconfig",
      "include directory": "/opt/arm64-builds/include",
      "lib directory": "/opt/arm64-builds/lib",
      "openblas configuration": "USE_64BITINT=0 DYNAMIC_ARCH=1 DYNAMIC_OLDER= NO_CBLAS= NO_LAPACK= NO_LAPACKE= NO_AFFINITY=1 USE_OPENMP= SANDYBRIDGE MAX_THREADS=64",
      "pc file directory": "/opt/arm64-builds/lib/pkgconfig"
    },
    "pybind11": {
      "name": "pybind11",
      "version": "2.12.0",
      "detection method": "config-tool",
      "include directory": "unknown"
    }
  },
  "Python Information": {
    "path": "/private/var/folders/l_/53d72k7j4zl9kj8fzncsd8hw0000gn/T/cibw-run-09g5pzn4/cp310-macosx_arm64/build/venv/bin/python",
    "version": "3.10"
  }
}
@m-reuter m-reuter added the defect A clear bug or issue that prevents SciPy from being installed or used as expected label May 8, 2024
This was referenced May 8, 2024
Merged
Open
@tylerjereddy
Copy link
Contributor

For the first part of the reproducer, on latest main at time of writing, I see this alongside NumPy 2.0.0rc1:

  (np.int32(1), np.int32(0))	1.0
  (np.int32(1), np.int32(0))	1.0
  (np.int32(0), np.int32(1))	1.0

sparse folks can let me know if they need additional backports for 1.13.1 if it needs a fix.

@tylerjereddy tylerjereddy added this to the 1.13.1 milestone May 9, 2024
@dschult
Copy link
Contributor

dschult commented May 10, 2024

This does indeed need a fix. It's a bug introduced in #19962 while trying to avoid multiple conversions between formats.
It "should be" a quick fix.

And, yes -- the fix should get additional backport for 1.13.1

Thank you @m-reuter for reporting it! Thank you @tylerjereddy for the backports!

@dschult dschult mentioned this issue May 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.sparse
Projects
None yet
Milestone
1.13.1
Development

Successfully merging a pull request may close this issue.

BUG: sparse: Fix summing duplicates for CSR/CSC creation from (data,coords) dschult/scipy
4 participants
@dschult @tylerjereddy @m-reuter @dschmitz89