Skip to content

BUG: Expr.__array_ufunc__ can't handle 0-dim array#1219

Open
Zeroto521 wants to merge 7 commits into
scipopt:masterfrom
Zeroto521:bug/issue-1218
Open

BUG: Expr.__array_ufunc__ can't handle 0-dim array#1219
Zeroto521 wants to merge 7 commits into
scipopt:masterfrom
Zeroto521:bug/issue-1218

Conversation

@Zeroto521
Copy link
Copy Markdown
Contributor

fix #1218

numpy automatically converts np.generic to a 0-dim array.

import numpy as np


class Expr:
    def __array_ufunc__(self, ufunc, method, *args, **kwargs):
        print("-" * 50)
        print(args)
        print([type(a) for a in args])
        print("-" * 50)

        return NotImplemented

    def __init__(self, value: int):
        self.value = value

    def __repr__(self):
        return f"Expr({self.value})"

    def __le__(self, other: int):
        return self.value <= other


if __name__ == "__main__":
    x = Expr(5)
    value = np.float64(5.0)
    print(value <= x)

    # --------------------------------------------------
    # (array(5.), Expr(5))  # <-- the first argument is a numpy array, not np.float64
    # [<class 'numpy.ndarray'>, <class '__main__.Expr'>]
    # --------------------------------------------------

Zeroto521 added 2 commits May 12, 2026 23:05
Adjust ExprLike.__call__ to only treat ndarrays with ndim >= 1 as array operands for MatrixExpr/MatrixGenExpr conversion. Convert np.generic and 0-dim np.ndarray values to native Python scalars via .item() to avoid __array_ufunc__ recursion with MatrixExpr/Expr. Also update the array filtering and in-place comment to reflect the new behavior.
Add test_np_generic_cmp_with_expr to cover issue scipopt#1218. The test verifies that comparisons between numpy generic scalars (np.float64) and expression objects produce the expected ExprCons string representations for both operand orders and for positive/negative values (x <= -value, x <= value, -value <= x, value <= x).
Add an Unreleased changelog entry noting that Expr.__array_ufunc__ couldn't handle 0-dimensional arrays.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes NumPy scalar/0-d array interactions with PySCIPOpt expressions by preventing ExprLike.__array_ufunc__ from treating 0-d arrays as matrix inputs, resolving the AttributeError reported in #1218 when comparing/negating np.float64 values against expressions.

Changes:

  • Update ExprLike.__array_ufunc__ to only route ndim >= 1 arrays through matrix-handling logic, and to coerce 0-d arrays (and np.generic) to native Python scalars via .item().
  • Add a regression test covering comparisons between np.float64 (including negated) and expression variables in both operand orders.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_expr.py Adds regression test for np.float64 comparison behavior with expressions (issue #1218).
src/pyscipopt/expr.pxi Adjusts __array_ufunc__ handling to coerce 0-d arrays to scalars and avoid incorrect matrix-ufunc routing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Joao-Dionisio
Copy link
Copy Markdown
Member

Hey @Zeroto521 , thank you thank you thank you! Some comments from my Claude, please see if any make sense.

1. Gate the 0-d unwrap on dtype.kind for symmetry with the matrix path

The 1-d+ path returns NotImplemented for non-numeric dtypes, but the new 0-d unwrap calls .item() unconditionally. So np.array(some_object, dtype=object) <= x would now silently extract the Python
object instead of returning NotImplemented. Probably never hit in practice, but easy to keep consistent:

args = [
    a.item()
    if isinstance(a, np.generic)
    or (isinstance(a, np.ndarray) and a.ndim == 0 and a.dtype.kind in "fiub")
    else a
    for a in args
]
  1. Broaden the regression test

Only <= is exercised, but the same code path serves >=, ==, and all the arithmetic ufuncs. Worth adding a couple more one-liners — == in particular is interesting because it produces a two-sided ExprCons:

  assert str(value >= x) == "ExprCons(Expr({Term(x): 1.0}), None, 5.0)"
  assert str(value == x) == "ExprCons(Expr({Term(x): 1.0}), 5.0, 5.0)"
  assert str(np.int64(5) <= x) == "ExprCons(Expr({Term(x): 1.0}), 5.0, None)"
  assert str(np.array(5) <= x) == "ExprCons(Expr({Term(x): 1.0}), 5.0, None)"
  1. Comment is slightly off

The comment mentions np.generic + MatrixExpr, but the bug being fixed is comparison ufuncs hitting .view(MatrixExprCons) on a plain ExprCons, not + recursion. Something like:

  # Numpy scalars and 0-d arrays must take the scalar dispatch path below,
  # not the matrix path — the matrix path calls .view(MatrixExprCons) on
  # the result, which fails for scalar comparisons returning ExprCons (#1218).

Nitpick: stray space at tests/test_expr.py:339 — str(x <= value ).

Zeroto521 added 4 commits May 13, 2026 21:41
Expand tests in tests/test_expr.py to cover additional comparisons between numpy scalars/arrays and Expr objects. Added assertions for <=, >= and == involving np.float64 and np.int64, and a test for a 0-dim numpy array to ensure ExprCons string representations are correct.
Update comments in tests/test_expr.py to explicitly note that comparisons involve numpy scalar (np.generic) and 0-dim arrays versus Variable instances. No test logic was changed—only comment text to improve clarity.
Add a unit test in tests/test_expr.py asserting that comparing 1 <= np.array(x) (a 0-dim Variable array) raises TypeError. This complements the existing 0-dim int array vs Variable test and helps catch regressions in comparison handling.
Update comment in src/pyscipopt/expr.pxi to explicitly mention that both np.generic and 0-dim np.ndarray are converted to native Python types to avoid __array_ufunc__ recursion with MatrixExpr/Expr. Documentation-only change; no functional edits.
Comment thread src/pyscipopt/expr.pxi
Comment on lines +213 to +224
# Convert `np.generic` and 0-dim `np.ndarray` to native Python types to stop
# __array_ufunc__ recursion from `np.generic + MatrixExpr/Expr` or
# `0-dim np.ndarray + MatrixExpr/Expr`.
args = [
a.item()
if (
isinstance(a, np.generic)
or (isinstance(a, np.ndarray) and a.ndim == 0)
)
else a
for a in args
]
Copy link
Copy Markdown
Contributor Author

@Zeroto521 Zeroto521 May 13, 2026

Choose a reason for hiding this comment

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

In this case, 1 <= np.array(x) gets recursion and crushes the Python kernel.

The 1-d+ path returns NotImplemented for non-numeric dtypes, but the new 0-d unwrap calls .item() unconditionally. So np.array(some_object, dtype=object) <= x would now silently extract the Python
object instead of returning NotImplemented. Probably never hit in practice, but easy to keep consistent:

args = [
    a.item()
    if isinstance(a, np.generic)
    or (isinstance(a, np.ndarray) and a.ndim == 0 and a.dtype.kind in "fiub")
    else a
    for a in args
]

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6.2.0 no longer handles values that are numpy.float64 in constraint comparisons being negated

3 participants