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.

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

Fix qml.math.get_interface for scipy input #7015

Merged
merged 18 commits into from
Feb 28, 2025
Merged

Conversation

andrijapau
Copy link
Contributor

@andrijapau andrijapau commented Feb 26, 2025

Context:

qml.math.get_interface defaults incorrectly to "numpy" when the len(input) > 1 and the input is not one of the supported ML frameworks. This was specifically noticed for inputs containing scipy csr_matrix.

Description of the Change:

A few things were done,

Source-code:

  • Enhance get_interface to handle which interface is returned.
  • In multi_dispatch.py theres no need to use ar.numpy since were already imported autograd.numpy as np. So we can clean that up.

Tests:

  • Add tests to cover bug.
  • Add qml.math.coerce tests for jax, autograd and scipy (even though they should trivially do nothing because of,
ar.register_function(<trivial interface>, "coerce", lambda x: x)

Benefits: Correct code.

Possible Drawbacks: None identified.

Related GitHub Issues: Fixes #7012

[sc-85330]

This comment was marked as resolved.

@JerryChen97 JerryChen97 self-requested a review February 26, 2025 20:37
Copy link
Contributor

@JerryChen97 JerryChen97 left a comment

Choose a reason for hiding this comment

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

math.dot might need to have scipy branch very first since this interface does not have quite some things in numpy

@JerryChen97
Copy link
Contributor

JerryChen97 commented Feb 26, 2025

https://github.com/PennyLaneAI/pennylane/actions/runs/13552851763/job/37880423118?pr=7015

It's expected that TestQubitUnitaryCSR failed (since one of my sparse PR recently introduced sparse dot into two_qubit_decomposition). But quite a surprise to me that very much old tests in test_prod.py failed as well! Does this mean that previous sparse matrix dot operations have been all the time dispatched back to numpy??

@JerryChen97
Copy link
Contributor

Also not sure how hard it could be to make dot(np.array, scipy.sparse.sparray | spmatrix) compatible here (fingers crossed)
In the new apply_operation I only use default dispatches of numpy and scipy for operator @ so no issue. But I guess it's slightly different from maintaining in qml.math.

@JerryChen97
Copy link
Contributor

https://github.com/PennyLaneAI/pennylane/actions/runs/13552851763/job/37880423118?pr=7015

It's expected that TestQubitUnitaryCSR failed (since one of my sparse PR recently introduced sparse dot into two_qubit_decomposition). But quite a surprise to me that very much old tests in test_prod.py failed as well! Does this mean that previous sparse matrix dot operations have been all the time dispatched back to numpy??

I just manually checked qml.math.dot(csr, csr) on master branch and it's working well to return a csr. I guess there's some scheme here to bypass and get csr @ csr directly.

@JerryChen97 JerryChen97 added the bug 🐛 Something isn't working label Feb 26, 2025
@andrijapau
Copy link
Contributor Author

The problem is that this is happening under the hood now.

import autoray
from autoray import numpy as np

>>> np.coerce(1., like="scipy")
ImportError: autoray couldn't find function 'coerce' for backend 'scipy'.

when normally it would be "numpy".

@albi3ro
Copy link
Contributor

albi3ro commented Feb 26, 2025

The problem is that this is happening under the hood now.

import autoray
from autoray import numpy as np

>>> np.coerce(1., like="scipy")
ImportError: autoray couldn't find function 'coerce' for backend 'scipy'.

when normally it would be "numpy".

We should just be able to do:

ar.register_function("scipy", "coerce", np.coerce)

in single_dispatch.py.

@JerryChen97
Copy link
Contributor

JerryChen97 commented Feb 26, 2025

The problem is that this is happening under the hood now.

import autoray
from autoray import numpy as np

>>> np.coerce(1., like="scipy")
ImportError: autoray couldn't find function 'coerce' for backend 'scipy'.

when normally it would be "numpy".

We should just be able to do:

ar.register_function("scipy", "coerce", np.coerce)

in single_dispatch.py.

It seems only the numpy from autoray can do coerce. Native numpy as in single_dispatch.py can't find its own coerce

>>> np.coerce
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/yushao.chen/Documents/pennylane/venv/lib/python3.11/site-packages/numpy/__init__.py", line 333, in __getattr__
    raise AttributeError("module {!r} has no attribute "
AttributeError: module 'numpy' has no attribute 'coerce'. Did you mean: 'source'?
>>> import autoray as ar
>>> ar.numpy.coerce
functools.partial(<function do at 0x7f26b0e69760>, 'coerce')

@andrijapau
Copy link
Contributor Author

andrijapau commented Feb 26, 2025

The problem is that this is happening under the hood now.

import autoray
from autoray import numpy as np

>>> np.coerce(1., like="scipy")
ImportError: autoray couldn't find function 'coerce' for backend 'scipy'.

when normally it would be "numpy".

We should just be able to do:

ar.register_function("scipy", "coerce", np.coerce)

in single_dispatch.py.

It seems only the numpy from autoray can do coerce. Native numpy as in single_dispatch.py can't find its own coerce

>>> np.coerce
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/yushao.chen/Documents/pennylane/venv/lib/python3.11/site-packages/numpy/__init__.py", line 333, in __getattr__
    raise AttributeError("module {!r} has no attribute "
AttributeError: module 'numpy' has no attribute 'coerce'. Did you mean: 'source'?
>>> import autoray as ar
>>> ar.numpy.coerce
functools.partial(<function do at 0x7f26b0e69760>, 'coerce')

Edit: Ignore this! It's recursive because they are both calling the same function.

Hmm, I did the following in that file,

def _builtins_coerce(x, like=None):
    return ar.numpy.coerce(x, like=like)

...
ar.register_function("builtins", "coerce", _builtins_coerce)
...

# -------------------------------- SciPy --------------------------------- #
# the following is required to ensure that SciPy sparse Hamiltonians passed to
# qml.SparseHamiltonian are not automatically 'unwrapped' to dense NumPy arrays.
...
ar.register_function("scipy", "coerce", ar.numpy.coerce)
...

since just adding the scipy one gave me the same ImportError instead for __builtins__. 😢 Now I am getting these errors (for the same tests that are failing now),

RuntimeError: Maximum recursion depth reached! This is likely due to nesting too many levels of composite operators. Try setting lazy=False when calling qml.sum, qml.prod, and qml.s_prod, or use the +, @, and * operators instead. Alternatively, you can periodically call qml.simplify on your operators.

@JerryChen97
Copy link
Contributor

np.dot doesn't work for np.dot(ndarray, scipy.sparse). But I feel this is a numpy issue not our fault.

>>> X
array([[0, 1],
       [1, 0]])
>>> b
<Compressed Sparse Row sparse matrix of dtype 'complex128'
        with 2 stored elements and shape (2, 2)>
>>> np.dot(X, b)
array([[<Compressed Sparse Row sparse matrix of dtype 'complex128'
                with 2 stored elements and shape (2, 2)>                 ,
        <Compressed Sparse Row sparse matrix of dtype 'complex128'
                with 2 stored elements and shape (2, 2)>                 ],
       [<Compressed Sparse Row sparse matrix of dtype 'complex128'
                with 2 stored elements and shape (2, 2)>                 ,
        <Compressed Sparse Row sparse matrix of dtype 'complex128'
                with 2 stored elements and shape (2, 2)>                 ]],
      dtype=object)

@albi3ro
Copy link
Contributor

albi3ro commented Feb 26, 2025

I just found that we register:

ar.register_function("numpy", "coerce", lambda x: x)

🤦‍♀️

@andrijapau
Copy link
Contributor Author

I just found that we register:

ar.register_function("numpy", "coerce", lambda x: x)

🤦‍♀️

Not just numpy,
image

Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (9260c26) to head (888ed43).
Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7015   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files         484      484           
  Lines       46249    46305   +56     
=======================================
+ Hits        46065    46122   +57     
+ Misses        184      183    -1     

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

@JerryChen97
Copy link
Contributor

JerryChen97 commented Feb 27, 2025

The problem is that this is happening under the hood now.

import autoray
from autoray import numpy as np

>>> np.coerce(1., like="scipy")
ImportError: autoray couldn't find function 'coerce' for backend 'scipy'.

when normally it would be "numpy".

We should just be able to do:

ar.register_function("scipy", "coerce", np.coerce)

in single_dispatch.py.

It seems only the numpy from autoray can do coerce. Native numpy as in single_dispatch.py can't find its own coerce

>>> np.coerce
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/yushao.chen/Documents/pennylane/venv/lib/python3.11/site-packages/numpy/__init__.py", line 333, in __getattr__
    raise AttributeError("module {!r} has no attribute "
AttributeError: module 'numpy' has no attribute 'coerce'. Did you mean: 'source'?
>>> import autoray as ar
>>> ar.numpy.coerce
functools.partial(<function do at 0x7f26b0e69760>, 'coerce')

Edit: Ignore this! It's recursive because they are both calling the same function.

Hmm, I did the following in that file,

def _builtins_coerce(x, like=None):
    return ar.numpy.coerce(x, like=like)

...
ar.register_function("builtins", "coerce", _builtins_coerce)
...

# -------------------------------- SciPy --------------------------------- #
# the following is required to ensure that SciPy sparse Hamiltonians passed to
# qml.SparseHamiltonian are not automatically 'unwrapped' to dense NumPy arrays.
...
ar.register_function("scipy", "coerce", ar.numpy.coerce)
...

since just adding the scipy one gave me the same ImportError instead for __builtins__. 😢 Now I am getting these errors (for the same tests that are failing now),

RuntimeError: Maximum recursion depth reached! This is likely due to nesting too many levels of composite operators. Try setting lazy=False when calling qml.sum, qml.prod, and qml.s_prod, or use the +, @, and * operators instead. Alternatively, you can periodically call qml.simplify on your operators.

I'm trying to understand, why eventually we successfully got rid of the recursive imports Error 😂

@JerryChen97 JerryChen97 self-requested a review February 27, 2025 15:17
Copy link
Contributor

@JerryChen97 JerryChen97 left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@JerryChen97
Copy link
Contributor

Oh no 💣 it seems still the same bug as before:

>>> a = csr_matrix([1])
>>> a
<Compressed Sparse Row sparse matrix of dtype 'int64'
        with 1 stored elements and shape (1, 1)>
>>> qml.math.coerce(a, like="scipy")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 81, in do
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 81, in do
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 81, in do
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  [Previous line repeated 990 more times]
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 79, in do
    backend = _choose_backend(fn, args, kwargs, like=like)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 402, in _choose_backend
    return _infer_auto(fn, args, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 93, in _default_infer_from_sig
    return _DISPATCHERS[fn](*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 1390, in default_dispatcher
    return infer_backend(args[0])
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yushao.chen/.local/lib/python3.11/site-packages/autoray/autoray.py", line 301, in infer_backend
    return _infer_class_backend_cached(array.__class__)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RecursionError: maximum recursion depth exceeded in comparison
>>> 

@JerryChen97
Copy link
Contributor

Also note that in test_functions.py we only had TestCoercion for tf and torch

@andrijapau andrijapau enabled auto-merge (squash) February 28, 2025 18:45
@andrijapau andrijapau merged commit 9b45754 into master Feb 28, 2025
46 checks passed
@andrijapau andrijapau deleted the fix-get-interface branch February 28, 2025 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] math.get_interface can't extract scipy from a list of sparse arrays/matrices
3 participants