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

Unable to create numpy int array from an EdgeList #614

Closed
mtreinish opened this issue May 19, 2022 · 5 comments · Fixed by #615
Closed

Unable to create numpy int array from an EdgeList #614

mtreinish opened this issue May 19, 2022 · 5 comments · Fixed by #615
Labels
bug Something isn't working
Milestone

Comments

@mtreinish
Copy link
Member

Information

  • retworkx version: main branch (since 71e34d1)
  • Python version: 3.10
  • Rust version: 1.60
  • Operating system: Linux

What is the current behavior?

Trying to create a numpy array from an EdgeList object fails

TypeError: int() argument must be a string, a bytes-like object or a real number, not 'retworkx.EdgeList'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/tmp/test_failure.py", line 5, in <module>
    print(np.asarray(res, dtype=np.uintp))
ValueError: setting an array element with a sequence.

I bisected this to 71e34d1 (#569). I'm not sure if this is caused by changes in the newer version of pyo3 or our minimal changes to the custom_vec_iter_impl macro (I'm leaning towards a pyo3 change).

What is the expected behavior?

calling np.asarray(g.edge_list, dtype=np.uintp) will create a numpy array for the edge list like:

[[0 1]
 [0 2]
 [0 3]
 [0 4]]

Steps to reproduce the problem

import retworkx
import numpy as np
g = retworkx.generators.directed_star_graph(5)
res = g.edge_list()
print(np.asarray(res, dtype=np.uintp))
@mtreinish mtreinish added the bug Something isn't working label May 19, 2022
@mtreinish
Copy link
Member Author

This is going to be a release blocking issue because this causes errors in qiskit-terra as it relies on being able to np.asarray(res, dtype=np.uintp) from graph objects.

@IvanIsCoding IvanIsCoding added this to the 0.12.0 milestone May 20, 2022
@IvanIsCoding
Copy link
Collaborator

IvanIsCoding commented May 20, 2022

A possible fix without downgrading PyO3 is to implement the __array__ method as described in the NumPy API. That way we can guarantee that np.asarray will always works for our custom types.

It is puzzling that bumping the PyO3 version broke our code, especially considering we did not update iterators.rs. We should add some tests for conversions that are important to us

@mtreinish
Copy link
Member Author

That's a good idea, that also would potentially avoid the iteration overhead too because we could go straight to a numpy array via the c api (not that array creation from our custom return types is a bottleneck for anything).

@mtreinish
Copy link
Member Author

I also reported this upstream to pyo3 in PyO3/pyo3#2392 one option we might want to look at is converting back to use pyproto with the PySequenceProtocol trait for the custom_vec_iter_impl macro https://github.com/Qiskit/retworkx/blob/7dc5182a61fdc67425038f5f8cf9bf4474c080ca/src/iterators.rs#L416-L533 and suppress the deprecation warnings. That is probably the simplest fix to start.

@georgios-ts
Copy link
Collaborator

The new pyo3 implementation fills mp_length slot but not sq_length. I'm guessing this causes numpy some trouble and can't identify our custom return types as sequence objects.

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 a pull request may close this issue.

3 participants