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

Comparison operator overloading doesn't work with sparse matrices #4819

Open
SteveDiamond opened this Issue May 6, 2015 · 7 comments

Comments

Projects
None yet
8 participants
@SteveDiamond

SteveDiamond commented May 6, 2015

I'm the developer of a modeling framework for optimization problems called cvxpy. I'd like to support operator overloading between scipy sparse matrices and cvxpy objects.

Currently operator overloading works for arithmetic operations, but I also need it to work for comparison operators. The issue is that all comparison operators route through the _inequality function in scipy/sparse/compressed.py, and there's no code path in that function that returns NotImplemented.

Could you add some code path that returns NotImplemented like in the arithmetic operators? That way the reflection will be called on the cvxpy object. For example, if A is a sparse matrix and x is a cvxpy variable, the expression A <= x will first call A.__le__(x). This needs to return NotImplemented so Python calls x.__ge__(A), which creates a cvxpy constraint object.

The same issues apply to the __eq__ method in scipy/sparse/compressed.py. There should be some code path that returns NotImplemented.

Another solution would be to check __array_priority__ like in Numpy. The object with the higher __array_priority__ would have its comparison operator called first.

@argriffing

This comment has been minimized.

Contributor

argriffing commented May 6, 2015

The currently active discussion at numpy/numpy#5844 looks related.

@SteveDiamond

This comment has been minimized.

SteveDiamond commented Jun 22, 2015

A clarification: what I really want is a clean way to make <scipy sparse matrix>.__binop__(<cvxpy object>) return NotImplemented for all binops. Ideally this would be via a flag in cvxpy objects.

@ghost

This comment has been minimized.

ghost commented Feb 24, 2017

I was solving this issue, so shall I include various cases for dense matrix, sparse matrix ,etc: in the binary operator function that are there in add , subtract functions or should I just raise the error.

@perimosocordiae

This comment has been minimized.

Member

perimosocordiae commented Feb 24, 2017

I'm not sure what you mean. Feel free to open a pull request with some partial fixes and mark it WIP. That way we can discuss the code directly.

@adbugger

This comment has been minimized.

Contributor

adbugger commented Feb 19, 2018

Hi, I'd like to work on this bug if nobody is working on it currently. The last activity seems to be a year ago?

If it's possible, a point in the right direction (in terms of what files to understand and what code to modify) would be a big help.

@ilayn

This comment has been minimized.

Member

ilayn commented Feb 19, 2018

If I understand correctly this has to be done at cvxpy side with either __array_priority__ (or __array_ufunc__ for NumPy 1.13+).

@jairideout

This comment has been minimized.

jairideout commented Sep 23, 2018

I'm interested in working on this issue if it isn't already being worked on. It looks like the last activity was in February. Is it okay for me to move forward with fixing the issue? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment