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

Allow passing numpy array to PyDiGraph::extend_from_edge_list #1125

Open
biswash47 opened this issue Feb 29, 2024 · 3 comments
Open

Allow passing numpy array to PyDiGraph::extend_from_edge_list #1125

biswash47 opened this issue Feb 29, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@biswash47
Copy link

What is the expected enhancement?

Per current api, one way to extend an existing graph is to use the PyDiGraph::extend_from_edge_list. This method expects a list of tuples. The enhancement would be extend this method so that it accepts a numpy array. This could possibly improve performance if the underlying memory array is directly accessed.

This issue came up in a discussion about the performance of qiskit.transpiler.CouplingMap object initialization: Qiskit/qiskit#11919

@biswash47 biswash47 changed the title Allow passing numpy arrays to PyDiGraph::extend_from_edge_list Allow passing numpy array to PyDiGraph::extend_from_edge_list Feb 29, 2024
@IvanIsCoding IvanIsCoding added the enhancement New feature or request label Mar 3, 2024
@IvanIsCoding
Copy link
Collaborator

I think this should be an OK addition, but it will need to be a separate method.

We had issues with conversions from ndarray -> Rust types before and that is why we have from_complex_adjacency_matrix and from_adjacency_matrix. So let's try to avoid those and just create a new method. Upstream users will need to adapt.

@jakelishman
Copy link
Member

Ivan: an alternative is to accept a Py<PyAny> in the slot and implement the conversion logic using the PyO3 traits manually (with a little bit of type checking). That's more complex from the implementation side, but potentially more convenient for users?

@IvanIsCoding
Copy link
Collaborator

Ivan: an alternative is to accept a Py<PyAny> in the slot and implement the conversion logic using the PyO3 traits manually (with a little bit of type checking). That's more complex from the implementation side, but potentially more convenient for users?

I think it is possible but I would like to keep the method straightforward. Nominally, I don't want to deal with two things:

  • Handling people passing Python objects that are Sequence[tuple[int, int]] but are not lists. I do not want to accidentally cast that to NumPy
  • Handling the different NumPy types. For even the slightest mismatch we get stuff like TypeError: argument 'matrix': type mismatch: from=uint8, to=float64 (this is like Add a method to make a graph from a complex matrix #411 but I called adjacency matrix with uint8 from NumPy)

So I'd rather implement a separate function and document well the specific ndarray format and type we want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants