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

feat: Add gate modifiers and additional supported gates to AutoQASM #958

Merged
merged 11 commits into from
May 6, 2024

Conversation

rmshaffer
Copy link
Contributor

@rmshaffer rmshaffer commented Apr 16, 2024

Issue #, if available:
Gate modifier support in AutoQASM: #957
Support U, GPhase, PRx gates in AutoQASM

Description of changes:
Add support for three additional keyword arguments to AutoQASM gates, for consistency with the Circuit class and associated gates (see #562):

  • control: QubitIdentifierType | Iterable[QubitIdentifierType] | None = None
  • control_state: BasisStateInput | None = None
  • power: float | None = None

Also add the gphase, u, and prx gate definitions for consistency with the main Braket SDK (see #799, #945).

Testing done:
tox passes. Tests added to cover new functionality.

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@rmshaffer rmshaffer self-assigned this Apr 16, 2024
Copy link

codecov bot commented Apr 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (72af596) to head (799891b).

Additional details and impacted files
@@                Coverage Diff                 @@
##           feature/autoqasm      #958   +/-   ##
==================================================
  Coverage            100.00%   100.00%           
==================================================
  Files                   173       172    -1     
  Lines                 10589     10568   -21     
  Branches               2264      2261    -3     
==================================================
- Hits                  10589     10568   -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rmshaffer rmshaffer marked this pull request as ready for review April 17, 2024 19:36
@rmshaffer rmshaffer requested a review from a team as a code owner April 17, 2024 19:36
@rmshaffer rmshaffer changed the title feat: Support control, control_state, power gate modifiers in AutoQASM feat: Add gate modifiers and additional gate support to AutoQASM Apr 17, 2024
@rmshaffer rmshaffer changed the title feat: Add gate modifiers and additional gate support to AutoQASM feat: Add gate modifiers and additional supported gates to AutoQASM Apr 17, 2024
Comment on lines +370 to +371
t(1, control=0)
t(2, control=[0, 1], control_state="10", power=0.123)
Copy link
Contributor

@ajberdy ajberdy Apr 18, 2024

Choose a reason for hiding this comment

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

Could be interesting to think of alternatives, like

aq.ctrl([0, 1], "10") @ aq.pow(0.123) @ t(2)

or

# allows controlling logical chunks of a program 
# (can we distribute the op effectively?)
with aq.ctrl(0):
    t(1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that there may be a more clever syntax for this. The approach in this PR is most consistent with what the Braket SDK does today (and also happens to be the easiest to implement, given how OQpy implements control modifiers). But we should keep thinking about this and consider whether there's something nicer to do.

control = [control]

if control_state is None:
control_state = [1] * len(control)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this logic duplicated in the BDK? I suppose the risk of it changing / getting out of sync is pretty minuscule if there's no bugs

Copy link
Contributor Author

@rmshaffer rmshaffer May 6, 2024

Choose a reason for hiding this comment

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

Yes, it's a duplicate of the logic here:

self._control_state = BasisState(
(1,) * len(control_set) if control_state is None else control_state,
len(control_set),
)

It would be difficult to re-use directly since we don't use the BDK Instruction class in AutoQASM. I do agree that the duplication is not ideal, but it seems very low-risk.

@rmshaffer rmshaffer merged commit d62e288 into feature/autoqasm May 6, 2024
23 checks passed
@rmshaffer rmshaffer deleted the rmshaffer/autoqasm-gate-modifiers branch May 6, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add support for gate modifiers (control, control_state, power) in AutoQASM
4 participants