-
Notifications
You must be signed in to change notification settings - Fork 22
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: Introduce run_multiple
method
#264
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 48 48
Lines 3791 3802 +11
Branches 927 930 +3
=========================================
+ Hits 3791 3802 +11 ☔ View full report in Codecov by Sentry. |
test/unit_tests/braket/default_simulator/test_density_matrix_simulator.py
Outdated
Show resolved
Hide resolved
test/unit_tests/braket/default_simulator/test_density_matrix_simulator.py
Outdated
Show resolved
Hide resolved
test/unit_tests/braket/default_simulator/test_state_vector_simulator.py
Outdated
Show resolved
Hide resolved
test/unit_tests/braket/default_simulator/test_state_vector_simulator.py
Outdated
Show resolved
Hide resolved
test/unit_tests/braket/default_simulator/test_density_matrix_simulator.py
Outdated
Show resolved
Hide resolved
expected_measurements = [[1, 0], [0, 1], [1, 1]] | ||
simulator = DensityMatrixSimulator() | ||
for result, payload_args, expected in zip( | ||
simulator.run_multiple(payloads, args=args), args, expected_measurements |
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.
aren't you missing a kwargs=kwargs
here?
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.
Ah I see what's going on. IMO this is pretty confusing, to allow shots
to be in either
test/unit_tests/braket/default_simulator/test_state_vector_simulator.py
Outdated
Show resolved
Hide resolved
Args: | ||
payloads (Sequence[Union[OQ3Program, AHSProgram, JaqcdProgram]]): The IR representations | ||
of the programs | ||
args (Optional[Sequence[Sequence[Any]]]): The positional args to include with |
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 I only submit one of args
and kwargs
? Is the intent that specifying shots
in kwargs
should override any value in args
?
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.
You can specify both, just as you can technically
device.run(circuit, 1000, shots=1000)
it'll just error out with something like
LocalSimulator.run() got multiple values for argument 'shots'
Issue #, if available:
Description of changes:
Introduces
run_multiple
method toBraketSimulator
to allow backends to leverage their own batching implementations. Will next publish SDK PR to make use of this interface.Testing done:
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.