-
Notifications
You must be signed in to change notification settings - Fork 196
Support executor + noise_learner_v3 schema v0.2 #2532
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
base: executor_preview
Are you sure you want to change the base?
Support executor + noise_learner_v3 schema v0.2 #2532
Conversation
| "python-dateutil>=2.8.0", | ||
| "ibm-platform-services>=0.22.6", | ||
| "ibm-quantum-schemas @ git+https://github.com/Qiskit/ibm-quantum-schemas.git@d579621311e62b5c342e804a605e4e4e96d88811#egg=ibm-quantum-schemas", | ||
| "ibm-quantum-schemas @ git+https://github.com/Qiskit/ibm-quantum-schemas.git@1b6b76605f482ae0e42cdf1d53c93fca36a53c67#egg=ibm-quantum-schemas", |
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.
TODO: this currently points to a feature branch. After this is merged, we should point this to a commit on main
|
|
||
|
|
||
| def noise_learner_v3_inputs_from_0_1( | ||
| def noise_learner_v3_inputs_from_0_2( |
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 function is not needed except in unit-tests. Does not look like production code. Should we move this into tests directory?
|
|
||
|
|
||
| def noise_learner_v3_result_to_0_1( | ||
| def noise_learner_v3_result_to_0_2( |
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 function is not needed except in unit-tests. Does not look like production code. Should we move this into tests directory?
| logger = logging.getLogger(__name__) | ||
|
|
||
| AVAILABLE_DECODERS = {"v0.1": noise_learner_v3_result_from_0_1} | ||
| AVAILABLE_DECODERS = {"v0.2": noise_learner_v3_result_from_0_2} |
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.
What is the idea of this dict? I could not see a reason to support multiple decoders as part of this PR.
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 is common for users to submit a job in one version of qiskit-ibm-runtime, and then wish to re-download the result by job id later in time (possibly months later) to look at it again. Without this dispatch table, they'd have to look around for a compatible qiskit-ibm-runtime version and forcefully downgrade in order to see their data. If they wanted to do some kind of A/B comparison of old data vs new data, they wouldn't be able to easily do it within a single Python environment.
In short, the user experience would effectively feel like a breaking change between minor versions in certain data analysis workflows.
This is not symmetric in that we do not need to maintain multiple encoders because this same argument doesn't apply. think @mtreinish may have advocated in the past to do so (I'll let him correct me), but whatever those reasons, they I imagine that they are different than the above.
|
Currently I am stuck at a transpiler test failing. Note: CI fails on something else, root-cause beeing Schema points to a pre-release version of qiskit. I guess this will fix itself once things get released. |
TsafrirA
left a comment
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.
Like yourself, I am also a bit puzzled by the backward compatibility here.
Generally speaking, I believe we wanted qiskit_ibm_runtime to be able to emit (at least a sub set of) previous schema versions (in case the server side doesn't support a given new version). Given this is not an officially released version of qiskit_ibm_runtime I am not sure if we want to be this strict, but it does create a race condition between this PR and the server side PR and deployment. Note that this branch is the only active interface with Executor and NoiseLearnerV3.
|
About decoders of quantum programs, be aware of this open PR: #2530 This PR is still open, so I'm note sure if it will be eventually approved and merged. |
Summary
ibm-quantum-schemaswill soon introduce new schema versionsv0.2for executor and noise-learner.This PR will use the new schema version in runtime-client to encode the input and decode the output for those programs.
Details and comments
I replaced v0.1 with v0.2, as I did not see a reason for still supporting v0.1 after v0.2 is released. Not sure if this assumption is correct, as there were some counter-indications in the structure of the code.
Fixes #