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: pulse programming support for OQC Lucy #191

Merged
merged 159 commits into from
Oct 24, 2023

Conversation

mudit2812
Copy link
Contributor

@mudit2812 mudit2812 commented Jul 26, 2023

Description of changes:
Adds access to the OQC device Lucy through PennyLane's pulse programming module.

Where it was simple to do so, the code was kept general for ease of adding additional devices later. However, some refactoring to remove hardcoded assumptions based on the Lucy device will certainly be necessary to add additional pulse-based devices.

  • adds ParametrizedEvolution as an allowed operation for the OQC Lucy device only
  • adds a dispatch of _translate_operation for converting a PennyLane ParameterizedEvolution to a Braket PulseGate
    • to facilitate this, because device-specific information regarding frames is needed for translation, a device kwarg is added to _translate_operation, which defaults to None
  • adds validation checks for parameters of the pulse where needed to ensure timely and clear error messages wrt invalid pulse inputs
  • adds a pulse_settings attribute (user-facing) that retrieves and displays some of the device specs (resonant frequency, connectivity, anharmonicity) that users might want in PL simulations
    • to facilitate this, several helper-functions are added to retrieve frame names rather than hardcoding them to use the Lucy format

Open questions:

  • Should a warning be raised if a user provides settings to use with a pulse to inform them that the settings will be ignored? Raising a warning would be inconsistent with vanilla PennyLane behaviour.

Testing done:

  • unit testing of the _translate_operation function for both constant amplitudes and amplitudes with a callable defining a pulse envelope
  • unit testing for the pulse_settings and the functions to access device frames
  • unit testing for errors raised in validation for invalid ParametrizedEvolution operators

Integration tests added that:

  1. check that the circuit is created and uploaded, and then cancelling the task (in the event that the device is unavailable)
  2. check that the circuit is uploaded and run (in the event the device is available)

Without pulse calibration data for the Lucy device, it is not possible to compare a pulse execution to an expected result. The integration tests currently run locally without raising an error.

Tests similar to these were run successfully before the device went offline for maintenance3. These tests specifically connect to and interact with the OQC Lucy device.

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.

lillian542 and others added 30 commits March 9, 2023 13:55
Also made plugin compatible with new return types specification (amazon-braket#153)

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
Co-authored-by: Christina Lee <chrissie.c.l@gmail.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Korbinian Kottmann <43949391+Qottmann@users.noreply.github.com>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: Cody Wang <speller26@gmail.com>
Co-authored-by: Korbinian Kottmann <43949391+Qottmann@users.noreply.github.com>
@christianbmadsen christianbmadsen requested a review from a team as a code owner September 28, 2023 15:51
@Qottmann Qottmann mentioned this pull request Oct 4, 2023
8 tasks
Copy link
Contributor

@jcjaskula-aws jcjaskula-aws left a comment

Choose a reason for hiding this comment

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

According to the latest tests we did, I believe that the phase need to adapt as this. I should be ok to approve it once we do this change.

lillian542 and others added 2 commits October 19, 2023 12:05
Co-authored-by: Jean-Christophe Jaskula <99367153+jcjaskula-aws@users.noreply.github.com>
kshitijc
kshitijc previously approved these changes Oct 19, 2023
# Create waveform for each pulse in `ParametrizedEvolution`
if callable(pulse.amplitude):
if pulse.amplitude == qml.pulse.constant:
amplitude = float(op.parameters[callable_index])
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this float or set complex to allow sending complex constant amplitudes? Below we are also not casting to any specifc dtype

jcjaskula-aws
jcjaskula-aws previously approved these changes Oct 20, 2023
Copy link
Contributor

@jcjaskula-aws jcjaskula-aws left a comment

Choose a reason for hiding this comment

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

last change float -> complex is fine and should not change anything.

kshitijc
kshitijc previously approved these changes Oct 20, 2023
@Qottmann Qottmann dismissed stale reviews from kshitijc and jcjaskula-aws via 6bac245 October 24, 2023 09:30
@Qottmann
Copy link
Contributor

There were conflicts in the imports of functools and typing with main. They are resolved and tests pass locally. Can we get another two approvals and merge?

@kshitijc kshitijc merged commit c505f94 into amazon-braket:main Oct 24, 2023
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.