-
Notifications
You must be signed in to change notification settings - Fork 34
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
Remove idle wires by default when separating the circuit #594
Conversation
Pull Request Test Coverage Report for Build 9229938335Details
💛 - Coveralls |
qubit_subsets = [ | ||
subset | ||
for subset in qubit_subsets | ||
if not (len(subset) == 1 and next(iter(subset)) in idle_wires) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to use next(iter(subset))
here because that is the standard way to get the "first" element from a set
.
""" | ||
|
||
subcircuits: dict[Hashable, QuantumCircuit] | ||
qubit_map: list[tuple[Hashable, int]] | ||
qubit_map: list[tuple[Hashable, int] | tuple[None, None]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out to be a lot easier to work with this if it's always a 2-tuple (which means you can always unpack it), which is which why I've used (None, None)
instead of None
.
j = circuit.find_bit(qubit).index | ||
partition_id = qubit_map[j][0] | ||
if partition_id is None: | ||
raise ValueError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you raise here because None
is Hashable
? Users could pass in a Sequence[None]
, and it would obey the Sequence[Hashable]
type hint, but be an invalid input if the None
s were associated with active qubits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you raise here because
None
is technicallyHashable
? Users could pass in aSequence[None]
, and it would obey theSequence[Hashable]
type hint, but be an invalid input?
Not quite; it's because a label of None
means "this qubit should be idle," but if this code path is triggered, it's because there is an instruction that uses the qubit in the circuit (i.e., it's not actually idle).
I wonder if the error message (on the following lines) could be clarified in any way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think it's a good message. This arises when a user passes None
as a label for an active qubit, and I think that's clear here.
I guess what I was getting at is that the user passing None
on an active qubit accidentally doesn't disobey the type hint, so raising a ValueError
is appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks nice, thanks! 🚀
Currently, if one has a 10-qubit circuit and only 2 qubits are used, then
partition_problem
will separate the input into 8 circuits that have a single unused qubit, plus one or two circuits (depending on connectivity) for the 2 qubits actually used. This changes the default to remove idle wires. It also means that the partition label ofNone
will be treated specially (it can only be used on an idle wire).Remaining action items