-
Notifications
You must be signed in to change notification settings - Fork 1k
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
No longer support create_calibration_program
and create_batch_program
#6442
No longer support create_calibration_program
and create_batch_program
#6442
Conversation
…/Cirq into u/smeeks/rm_unused_job_types
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6442 +/- ##
==========================================
- Coverage 97.82% 97.75% -0.07%
==========================================
Files 1115 1105 -10
Lines 97390 94897 -2493
==========================================
- Hits 95267 92771 -2496
- Misses 2123 2126 +3 ☔ View full report in Codecov by Sentry. |
Apologies, this PR kind of ballooned after tracing down all the dependencies for |
For more context: There are some idiosyncrasies that were done to remove all dependencies on removed code (e.g |
Ended up removing the idiosyncrasy noted above by changing the @wcourtney PTAL :) |
@@ -83,30 +81,18 @@ def validate_program( | |||
repetitions: Number of repetitions to run with each sweep. | |||
serializer: Serializer to use to serialize the circuits and sweeps. | |||
max_size: proto size limit to check against. | |||
|
|||
Raises: | |||
RuntimeError: if compiled proto is above the maximum size. | |||
""" |
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 looks like validation was removed entirely. I think we still want to serialize and validate the resulting size.
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.
Added back!
@@ -36,7 +36,7 @@ def _test_processor(processor: cg.engine.abstract_processor.AbstractProcessor): | |||
with pytest.raises(ValueError, match='Qubit not on device'): | |||
_ = processor.run(circuit, repetitions=100) | |||
circuit = cirq.Circuit(cirq.H(good_qubit), cirq.measure(good_qubit)) | |||
with pytest.raises(ValueError, match='Cannot serialize op'): | |||
with pytest.raises(ValueError, match='gate which is not supported'): |
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 should this message have changed 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.
Ah this broke after removing validation-- after adding validation back I was able to revert this change.
…/Cirq into u/smeeks/rm_unused_job_types
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.
So much deleted dead code!! Thanks for the cleanup 🧹
Restore proto files that are still used by internal code. This reverts small part of #6442.
This change removes support for engine type jobs
FocusedCalibration
andBatchProgram
, but keeps the interface for backwards compatibility