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

CQT-250 add barrier directives #370

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

juanboschero
Copy link
Collaborator

  • Barrier single qubit gate
  • Add barrier to circuit manager
  • Add tests

@juanboschero juanboschero requested review from rturrado and elenbaasc and removed request for rturrado November 5, 2024 14:59
Copy link
Contributor

@rturrado rturrado left a comment

Choose a reason for hiding this comment

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

  1. I don't think you need both rearrange_barriers and merge_barriers, but just rearrange_barriers:
  • You construct a list of lists of statements.
  • You keep a dictionary where the key is the qubit index, and the value is the statement index.
  • You walk the list of statements of your program in normal order.
  • The non-barrier statements go directly to the list of lists. For example, at one point, you may have [ [ H(0) ], [ CNOT(0, 1) ] ]. At that point, the dictionary would look like { 0: 1, 1: 1 }, i.e., both qubits 0 and 1 last appear in instruction 1.
  • Whenever you find a barrier you take it out to a separate ordered list.
  • When you have processed all the instructions, you walk the barrier list backwards, you get the index of the statement where that qubit was last used (just a look up in the dictionary), and you prepend the barrier to the list with that index (remember you have a list of lists).
  • When you are done processing the list of barriers, you just have to return a flattened version of the lists of lists, i.e., make it a list.
  1. I don't think merge_single_gate_qubits should be modified. Maybe just to call a rearrange_barriers function at the beginning of the code. But it could well request that barriers are already rearranged by the time you call this function.

opensquirrel/default_gates.py Outdated Show resolved Hide resolved
opensquirrel/default_gates.py Outdated Show resolved Hide resolved
opensquirrel/ir.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
test/test_circuit_builder.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
Copy link
Contributor

@rturrado rturrado left a comment

Choose a reason for hiding this comment

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

Nice!

  • We only have one merge_barriers function, not a rearrange_barriers anymore.
  • And merge_single_qubit_gates does not change.

I still think there are a few details to have a look at in the code.

test/test_circuit_builder.py Outdated Show resolved Hide resolved
test/test_circuit_builder.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
Juan Boschero and others added 3 commits November 8, 2024 08:31
Copy link
Contributor

@rturrado rturrado left a comment

Choose a reason for hiding this comment

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

Getting there.

Comment on lines 89 to 90
def _flatten_list(list_to_flatten: list[list[Any]]) -> list[Any]:
return functools.reduce(operator.iadd, list_to_flatten, [])
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add this function to a utils/list.py file (with a flatten_list name). This is a function that is probably going to be used from many places in the code, so it is useful to have it in a utilities file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename utils/list_utils.py to utils/list.py.

test/merger/test_merger.py Outdated Show resolved Hide resolved
test/merger/test_merger.py Outdated Show resolved Hide resolved
test/merger/test_merger.py Outdated Show resolved Hide resolved
Comment on lines +191 to +192
barrier q[1]
Anonymous gate: BlochSphereRotation(Qubit[0], axis=[ 0.65465 -0.37796 0.65465], angle=-2.41886, phase=1.5708)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why doesn't the anonymous gate go through barrier q[1]? barrier q[1] is not linked to barrier q[0].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Barrier 1 is merged with the first barrier 0 because it is more efficient to do so. As you said, it does not have anything to do with the last bit, so it will try to be scheduled earlier in the process than later. This is the strategy we have been implicitly using for all our other merges.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@juanboschero: I apologize for going back on the decision on optimizing the instructions with respect to the barriers. Optimization/reordering should be part of a separate pass, so I want to keep it disentangled here as well. So for now we stick with the less efficient merge_single_qubit_gates.

Copy link
Contributor

Choose a reason for hiding this comment

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

Barrier 1 is merged with the first barrier 0 because it is more efficient to do so.

Can you explain that "because it is more efficient to do so"?

test/merger/test_merger.py Outdated Show resolved Hide resolved
""",
),
],
ids=["initial-barrier", "cnot", "unpacking-usecase", "4-qubit", "barrier-only"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's think a bit more about this names:

  • "anonymous_gate"
  • "CNOT_cannot_go_through_a_group_of_linked_barriers"
  • "X_cannot_go_through_a_group_of_linked_barriers"
  • "circuit_with_4_qubits"
  • I don't know that the point of this last test is.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so according to your answer above, that last test should be called something like "circuit_with_barriers_and_CNOT_but_no_other_instructions".

opensquirrel/merger/general_merger.py Outdated Show resolved Hide resolved
opensquirrel/merger/general_merger.py Show resolved Hide resolved
return functools.reduce(operator.iadd, list_to_flatten, [])


def merge_barriers(statement_list: list[Statement]) -> list[Statement]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a problem with this code, and it shows in the first test, initial-barrier.
If we have something like:

barrier q[0]
H q[1]
barrier q[1]
H q[0]

We should end up with:

H q[1]
barrier q[0]
H q[0]
barrier q[1]

Right? Correct me if I'm wrong. Because H q[0] can go before barrier q[1].

However, because of the way this code works, we end up with:

H q[1]
barrier q[0]
barrier q[1]
H q[0]

Why does the code behave like that? Because, even if barrier q[0] and barrier q[1] are not linked in a beginning, once the code treats barrier q[1], it creates a group of linked barriers, over which "H q[0]" cannot go through.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, H q[0] can go before barrier q[1] but it goes against the strategy of merging the barriers. The most important functionality we should strive for is to make sure that these barrier walls across our circuit are correct (and also optimal as is the case above).

Copy link
Collaborator

Choose a reason for hiding this comment

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

At face value, the following result is permitted, given the meaning of the barriers:

H q[1]
barrier q[0]
H q[0]
barrier q[1]

including optimization this would become:

H q[1]
barrier q[0]
barrier q[1]
H q[0]

But since both restructurings influence the order of statements and such processing should be contained within a (rudimentary) Scheduling pass, which is not part of OpenSquirrel atm, the resulting expected result should be:

barrier q[0]
H q[1]
barrier q[1]
H q[0]

(basically no change... it couldn't be easier 😅 :)

Copy link
Contributor

@rturrado rturrado Nov 8, 2024

Choose a reason for hiding this comment

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

Yes, H q[0] can go before barrier q[1] but it goes against the strategy of merging the barriers.

Can you explain what's "the strategy of merging the barriers"?

The most important functionality we should strive for is to make sure that these barrier walls across our circuit are correct (and also optimal as is the case above).

But there is no barrier wall in the input code. There are two separate barriers. The only reason H q[0] cannot go before barrier q[1] is that (because a misfunction in the algorithm), by the time H q[0] tries to go before barrier q[1], the algorithm has built a group of barriers barrier q[0]; barrier q[1], i.e., the algorithm has linked those two barriers together (as if they were already together in the input program; only that that's not the case), and then H q[0] stays put.

juanboschero and others added 2 commits November 8, 2024 15:23
Co-authored-by: Roberto Turrado Camblor <rturrado@gmail.com>
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