Open biswash47 opened 6 months ago
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.
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?
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:
Sequence[tuple[int, int]]
but are not lists. I do not want to accidentally cast that to NumPyTypeError: argument 'matrix': type mismatch: from=uint8, to=float64
(this is like #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
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: https://github.com/Qiskit/qiskit/issues/11919