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.

Sign up for GitHub

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

Jump to bottom

Support for externally linked gates #182

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Support for externally linked gates #182

wants to merge 9 commits into from

Conversation

Sola85
Copy link

@Sola85 Sola85 commented Nov 5, 2024 •
edited
Loading

This is the companion PR to qBraid/pyqasm#59 for externally linked gates, as suggested here #167.

This is probably not ready to merge, but I figured it's easier if I get some feedback first. With both changes, external gates work in qbraid-qir, at least for my use-case.

If both PRs are merged, one can call e.g. qasm3_to_qir(qasm_str, external_gates=["mygate"]).

Then pyqasm does not unroll "mygate", but leaves it as a node in the AST. qbraid-qir then takes this node and converts it to an externally linked qir function. There are serveral ways this could be done (@christian512 originally suggested that the user passes a pyqir.Function that the external gate should be mapped to, alternatively the user could potentially also just pass the name of the desired qir function that the gate should be mapped to). Instead for now I chose to create the new qir function dynamically based on the arguments of the original gate and with the fixed name __quantum__qis__<GateName>__body, but this strategy could of course still be changed.

With qasm3_to_qir(qasm_str, external_gates=["mygate"]), the following QASM would then be mapped to the IR below:

OPENQASM 3.0;
include "stdgates.inc";
gate mygate(p0) _gate_q_0, _gate_q_1 {
  h _gate_q_1;
  cx _gate_q_0, _gate_q_1;
  rz(p0) _gate_q_1;
  cx _gate_q_0, _gate_q_1;
  h _gate_q_1;
}
bit[2] c;
qubit[2] q;
ry(pi/2) q[0];
mygate(pi/2) q[0], q[1];
entry:
  call void @__quantum__rt__initialize(i8* null)
  call void @__quantum__qis__ry__body(double 0x3FF921FB54442D18, %Qubit* null)
  call void @__quantum__qis__mygate__body(double 0x3FF921FB54442D18, %Qubit* null, %Qubit* inttoptr (i64 1 to %Qubit*))
  call void @__quantum__rt__result_record_output(%Result* null, i8* null)
  call void @__quantum__rt__result_record_output(%Result* inttoptr (i64 1 to %Result*), i8* null)
  ret void

The original _visit_basic_gate_operation had some logic where the gate was applied to multiple subsets of qubits

for i in range(0, len(op_qubits), op_qubit_count):
    # we apply the gate on the qubit subset linearly
    qubit_subset = op_qubits[i : i + op_qubit_count]
    ...

which I did not replicate in _visit_external_gate_operation since I did not understand this logic and could not trigger its execution. This might be be something that still requires change.

I also did not yet write any tests because most of the existing tests where failing due to import errors of pyqasm.pyqasm (I assume you are currently in the process of changing how pyqasm gets imported?). If the rest of this PR is good, I can look into this again and add tests for the external gates.

Closes #167

@TheGupta2012
Copy link
Collaborator

TheGupta2012 commented Nov 5, 2024 •
edited
Loading

Hi @Sola85! Thanks for your contributions and it looks like the changes are on the right track.

  • I feel that the naming convention __quantum__qis__<Gate_name>__body is fine for now and creating the function object dynamically seems like a good approach
  • The logic for applying gates to a subset of qubits was to ensure we cater to gate applications like - cx q[0], q[1], q[2], q[3] or h q[0], q[1], q[2] :
    • In case 1, cx is applied as cx q[0], q[1]; cx q[2], q[3]; in QIR form. Here, op_qubit_count is 2 and len(op_qubits) would be 4
    • In case 2, h is applied as h q[0]; h q[1]; in QIR form. Here, op_qubit_count is 1 and the len(op_qubits) is 2

I'll have to confirm whether this is still required( as we are now fully unrolling the qasm ), but this was the motivation behind the code that you mentioned. This would require you to apply an N qubit external gate in groups of N qubits and update the gate application code.

  • pyqasm is undergoing a lot of rapid changes and we are set to release a new version probably this week. Once that is done, I will update the requirements and fix the tests so that you can take it forward from there.

Besides this, I'll post any implementation specific comments in the diff.
Super glad to see you contribute to qBraid!

@Sola85
Copy link
Author

Sola85 commented Nov 5, 2024

The logic for applying gates to a subset of qubits was to ensure we cater to gate applications like - cx q[0], q[1], q[2], q[3] or h q[0], q[1], q[2]

Thats what I had assumed the logic was for. I tried a gate like h q[0], q[1], q[2] but I think this was already unrolled earlier, because I only ever saw that op_qubit_count was equal to len(op_qubits) for all examples I tried.

pyqasm is undergoing a lot of rapid changes and we are set to release a new version probably this week. Once that is done, I will update the requirements and fix the tests so that you can take it forward from there.

Sounds good! Then I'll wait for the update.

@TheGupta2012
Copy link
Collaborator

Hey, I've updated the pyqasm dependency to point to the latest release i.e. v0.0.3. You can rebase the PR and continue the work. Let me know if you face any issues!

@Sola85
Copy link
Author

Sola85 commented Nov 7, 2024

I added the same tests that I added in pyqasm (external u3 gate and external custom gates). The tests are currently failing in your pipeline because the pipeline doesn't know about my pyqir PR. Locally they pass.

These new tests didn't fit into the existing check_*_op() functions, so I created a new check_generic_gate_op, that can deal with arbitrary rotation angles and arbitrary qubit numbers, but does not take unrolling into account. Maybe this function is useful for the other tests too.

@TheGupta2012
Copy link
Collaborator

I think the changes on this PR are fine. Should be good to go once pyqasm changes are released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheGupta2012 TheGupta2012 TheGupta2012 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] External Function Calls
2 participants
@Sola85 @TheGupta2012