-
Notifications
You must be signed in to change notification settings - Fork 7
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
feature: Support for Braket Direct Reservations #87
feature: Support for Braket Direct Reservations #87
Conversation
Also, Added this function to demonstrate that we can add actions or assertions after starting reservation and before stopping reservation! I hope this makes sense. In julia, instead of AbstractContextManager which is available in python, we can use
|
Tested Locally:
|
src/device.jl
Outdated
""" | ||
DirectReservation(device::Union{Device, String, Nothing}, reservation_arn::Union{String, Nothing}) | ||
|
||
A context manager that adjusts AwsQuantumTasks generated within the context to utilize a reservation ARN |
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.
Why no doc link here for AwsQuantumTask
?
src/device.jl
Outdated
reserved device within the specified start and end times. | ||
|
||
Arguments: | ||
- device (Device | String | Nothing): The Braket device for which you possess a reservation ARN, or |
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.
Device
here can also use a doc link
src/device.jl
Outdated
Arguments: | ||
- device (Device | String | Nothing): The Braket device for which you possess a reservation ARN, or | ||
alternatively, the device ARN. | ||
- reservation_arn (String | Nothing): The Braket Direct reservation ARN to be implemented for all |
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 doesn't match the actual struct field types
src/device.jl
Outdated
# Start reservation function | ||
function start_reservation!(state::DirectReservation) | ||
if state.is_active | ||
error("Another reservation is already active.") |
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.
Why not include the reservation_arn
in this error message?
src/device.jl
Outdated
try | ||
func() | ||
catch e | ||
error("Error during reservation with device ARN $(reservation.device_arn): $(e)") |
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.
Why not show the reservation ARN in this error message as well?
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 might also make sense to create a new page in docs/src
for reservations, since they're a pretty advanced feature that need some explanation. This could be a stub for now to be fleshed out later.
Other than a last few small changes this looks pretty ready to go to me. |
Local Tests for Direct Reservations:
|
@@ -0,0 +1,8 @@ | |||
# Reservations | |||
|
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.
Should probably have some flavour text here explaining what reservations are and why anyone would use them.
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.
Please checkout the documentation.
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.
Can be merged into a feature branch (should add more integration tests that are difficult for UH participants to do themselves)
Resolving Issue #80
Description of changes: Amazon Braket now has the Braket Direct feature, which allows users to reserve certain Direct-only devices or access experimental capabilities of devices. Implementation of Python SDK? https://github.com/amazon-braket/amazon-braket-sdk-python/blob/main/src/braket/aws/direct_reservations. This allows users to provide a reservation arn to access their reservation, as in the BDK.
I will add docstrings and more tests, later today!
Testing done:
Note: I am new to git-secrets, so please bear with me!
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
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.