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

Introduce the "shots" parameter to 'sumbit' function of the 'Target' object #542

Conversation

ArthurKamalov
Copy link
Contributor

@ArthurKamalov ArthurKamalov commented Dec 22, 2023

This PR introduces new parameter short for a number of repeats for Target.submit function.

Things to do:

  • Complete changing signature for all derived classes.
  • Add more tests.
  • Find out why the x-client-cpu: x64 field is removed in the new test recordings.

@ArthurKamalov ArthurKamalov force-pushed the arturk/add_shots_param_to_target_submit branch from dfb54f9 to 4341bb7 Compare December 22, 2023 23:12
@ArthurKamalov ArthurKamalov force-pushed the arturk/add_shots_param_to_target_submit branch from 4341bb7 to 98856c8 Compare December 23, 2023 00:02
@ArthurKamalov ArthurKamalov force-pushed the arturk/add_shots_param_to_target_submit branch from 232d002 to 8e394e8 Compare January 3, 2024 23:01
Copy link
Contributor

@kikomiss kikomiss left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for working on this!

Once we add warning to microsoft.elements.dft and solver targets that the shots parameter is ignored (please, pull the latest from main) the work will be complete!

azure-quantum/azure/quantum/target/ionq.py Outdated Show resolved Hide resolved
azure-quantum/azure/quantum/target/microsoft/target.py Outdated Show resolved Hide resolved
azure-quantum/azure/quantum/target/quantinuum.py Outdated Show resolved Hide resolved
azure-quantum/azure/quantum/target/quantinuum.py Outdated Show resolved Hide resolved
@kikomiss kikomiss changed the base branch from main to users/kikomiss/features/qiskit-cirq-init-with-workspace-and-qsharp-lang January 5, 2024 02:43
ArturKamalov added 3 commits January 9, 2024 12:50
* The num_shots option for a target class now is checked by a helper function.
* New warning is printed on passing shots and num_shots together.
* Small fixes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kikomiss , Note: The Quantinuum backend will not print deprecation message for passing the 'count' option because its own parameter name for shots is also count, so we should not prevent user from specifying it, as any other "Quntinuum-specific" option.

* Fix pascal target
* add tests.
@ArthurKamalov ArthurKamalov marked this pull request as ready for review January 9, 2024 21:29
@ArthurKamalov ArthurKamalov requested a review from a team as a code owner January 9, 2024 21:29

def test_job_submit_pasqal_dict_input_params(self) -> None:
num_runs = 150
result = self._run_job(TEST_PULSER, input_params={"count": num_runs})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kikomiss, t it seems that Pascal targets now expect count option instead of runs (At least on prod).

@@ -53,7 +55,10 @@ def __init__(

@classmethod
def _default_options(cls):
return Options(count=500, targetCapability="BasicExecution")
other_options = {
cls._SHOTS_PARAM_NAME: 500
Copy link
Contributor

Choose a reason for hiding this comment

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

we've been setting the default options through _DEFAULT_SHOTS_COUNT earlier, we should probably follow the same pattern here.

Copy link
Contributor Author

@ArthurKamalov ArthurKamalov Jan 10, 2024

Choose a reason for hiding this comment

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

Yeah, I just thought it may be ok to keep it as literal since it is currently used only in one place, but I guess it wouldn't hurt🙂

if isinstance(input_params, InputParams):
typed_input_params = input_params
input_params = {
"runs": typed_input_params.runs,
self.__class__._SHOTS_PARAM_NAME: typed_input_params.runs,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems so. As I noted here #542 (comment), provider does not respond on runs option anymore, and when job is submitted, it falls back to Pascal's default values and returns "count" in details of a result job.

…-and-qsharp-lang' into arturk/add_shots_param_to_target_submit
@ArthurKamalov ArthurKamalov merged commit 761eb39 into users/kikomiss/features/qiskit-cirq-init-with-workspace-and-qsharp-lang Jan 10, 2024
2 checks passed
@ArthurKamalov ArthurKamalov deleted the arturk/add_shots_param_to_target_submit branch January 10, 2024 01:34
kikomiss added a commit that referenced this pull request Jan 10, 2024
…object (#542)

* Introduce the "shots" parameter to 'sumbit' function of the 'Target' object
* Remove num_shots from kwargs.
* Add warning on passing shots for estimator
* Add 'shots' option for qiskit backends.
* Refactor target shots option
* The num_shots option for a target class now is checked by a helper function.
* New warning is printed on passing shots and num_shots together.
* Allow provider-specific shots options.
* Fix pascal target
* add tests.
* Add back all possible shots params

---------

Co-authored-by: ArturKamalov <v-akamalov@microsoft.com>
Co-authored-by: kikomiss <144282031+kikomiss@users.noreply.github.com>
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.

None yet

2 participants