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

Update gate_map.py to allow nonstandard coupling graph sizes #7494

Closed
wants to merge 1 commit into from
Closed

Update gate_map.py to allow nonstandard coupling graph sizes #7494

wants to merge 1 commit into from

Conversation

evmckinney9
Copy link
Contributor

Summary

I found this error when trying to configure my own custom coupling graphs. The gate_map plot will always be blank unless the number of qubits matches a known grid layout in visualization/gate_map.py in the qubit_coordinates_map dictionary. A suggestion to fix this is to just round up to the nearest hard-coded grid size but don't print all the qubits. See #3692.

Details and comments

This doesn't make the prettiest pictures but it is better than blank plots.

An example using num_qubits=5, which is normally allowed.

LNN4 = ConfigurableFakeBackend(name='fake_lnn4', n_qubits=5, coupling_map=cmap, basis_gates=basis_gates)
plot_gate_map(LNN4, plot_directed=True)

image

And an example using num_qubits=4, which is using the grid defintion of 5, but not including the 5th qubit in the picture.

LNN4 = ConfigurableFakeBackend(name='fake_lnn4', n_qubits=4, coupling_map=cmap, basis_gates=basis_gates)
plot_gate_map(LNN4, plot_directed=True)

image

@CLAassistant
Copy link

CLAassistant commented Jan 6, 2022

CLA assistant check
All committers have signed the CLA.

@evmckinney9
Copy link
Contributor Author

I'm sure you could come up with a way to programmatically create qubit_coordinates_map, but I'll leave that to someone else.

@evmckinney9
Copy link
Contributor Author

evmckinney9 commented Jan 7, 2022

You can also add a default parameter qubit_coordinates=None to plot_gate_map().
Then do this check

#don't reference dictionary if provided as a parameter
if qubit_coordinates is None:
    qubit_coordinates = qubit_coordinates_map.get(num_qubits)

before passing qubit_coordinates to plot_coupling_map() allows you to override this funky solution with your own like

plot_gate_map(backend, plot_directed=True, qubit_coordinates=backend.qubit_coordinates)

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Thanks for starting this. I've always thought this function needs to be updated to be more general. Just hardcoding a layout that IBM uses for it's hardware really doesn't scale and prevents this from really being used with any backend. So having something here to fix this is great. Looking at this I had two thoughts, the first is we should probably add a qubit_layout kwarg to enable users to specify their own layout. While we totally should have a default layout generated if a user doesn't provide one, but having the option will give people flexibility in the output visualization.

The second is I'm not sure I agree with the approach in here of using the first n positions of the next largest layout we have. The connectivity of the coupling graphs could be completely different and the layout might not work well. It also doesn't fix the case where we have a coupling graph larger than any of the hard coded layouts. For example if you tried to run it with a coupling map created like: CouplingMap.from_heavy_hex(53). I wonder if we can leverage the layout and visualization functions in retworkx (which is used for the graph inside the CouplingMap class) here instead. Something like:

    if qubit_coordinates is None:
        qubit_coordinates = retworkx.spring_layout(coupling_map.graph, k=1.0, seed=1234)

or modifying plot_coupling_map() to use retworkx's mpl_draw() internally instead. https://qiskit.org/documentation/retworkx/apiref/retworkx.visualization.mpl_draw.html
This has the advantage of working for any graph and at least attempts to make a layout. We can play with the other layout functions in retworkx too: https://qiskit.org/documentation/retworkx/api.html#layout-functions to find something that works well (although I suspect we might want a planar layout method which doesn't exist yet: Qiskit/rustworkx#438 )

@evmckinney9
Copy link
Contributor Author

The connectivity of the coupling graphs could be completely different and the layout might not work well. It also doesn't fix the case where we have a coupling graph larger than any of the hard coded layouts.

I definitely agree that what I wrote is not a comprehensive solution but I figured it was at least a step in the right direction moving away from the hardcoded layouts. Ultimately, specifying qubit_coordinates seems to work well but retworkx looks like a good way to fill in the default cases.

Also note that these visualization methods will break for BackendV2, since they call backend.configuration(). Not sure what is being planned for compatibility moving forwards more than just wrapping necessary things in if statements such as

if isinstance(backend, BackendV2):
    num_qubits = backend.num_qubits
    coupling_map = backend.plot_coupling_map
else:
    config = backend.configuration()
    num_qubits = config.n_qubits
    coupling_map = config.coupling_map

@mtreinish
Copy link
Member

Yeah, we'll need to update the logic here. Typically I'd try to use the .version attribute of backends to get the interface version. A nice helper method for this is being added in #7563 so you can do something like:

from qiskit.utils.backend_utils import _get_backend_interface_version
backend_version = _get_backend_interface_version(backend)
if backend_version <= 1:
    config = backend.configuration()
    num_qubits = config.n_qubits
    coupling_map = config.coupling_map
else:
    num_qubits = backend.num_qubits
    coupling_map = backend.coupling_map

@mtreinish
Copy link
Member

I'm going to close this as stale, thanks for getting this started but there was some last minute urgency that came up for me around fixing this and I wanted to get this fixed for the next release (which should hopefully be releasing soon). So I took the ideas I proposed here around using retworkx and opened #7814 to provide an alternative fix. I hope you don't mind. If you think #7814 isn't sufficient or there is more to discuss on this PR please feel free to reopen this.

@mtreinish mtreinish closed this Mar 24, 2022
@evmckinney9
Copy link
Contributor Author

Sounds good, I have just been using retworkx instead now too. Thanks!

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.

3 participants