feat: Instruction classes and execution results now support the pickle module #1795
Conversation
|
| Report | Tue, July 30, 2024 at 17:51:13 UTC |
| Project | pyquil |
| Branch | 1719-more-pickleable-types |
| Testbed | ci-runner-linux |
Click to view all benchmark results
| Benchmark | Latency | Latency Results nanoseconds (ns) | (Δ%) | Latency Upper Boundary nanoseconds (ns) | (%) |
|---|---|---|---|
| test/benchmarks/test_program.py::test_copy_everything_except_instructions | ✅ (view plot) | 10,348,544,208.00 (+0.54%) | 10,536,846,752.01 (98.21%) |
| test/benchmarks/test_program.py::test_instructions | ✅ (view plot) | 3,896,421,725.20 (-0.73%) | 4,051,015,766.66 (96.18%) |
| test/benchmarks/test_program.py::test_iteration | ✅ (view plot) | 3,926,822,893.80 (-0.25%) | 4,063,221,555.28 (96.64%) |
Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help
test/unit/test_qpu.py
Outdated
| # result_data=ResultData.from_qvm( | ||
| # QVMResultData.from_memory_map({ | ||
| # "int": RegisterData.from_i8([[1, 2, 3], [4, 5, 6]]), | ||
| # "float": RegisterData.from_f64([[0.1, 0.2, 0.3], [0.4, 0.5, 0.6]]) | ||
| # }) | ||
| # ) |
test/unit/test_qpu.py
Outdated
| def test_pickle_qpu_execute_response(mock_encrypted_program): | ||
| response = QPUExecuteResponse( | ||
| job_id="some-job-id", | ||
| _executable=mock_encrypted_program, | ||
| execution_options=ExecutionOptions.default() | ||
| ) | ||
| pickled_response = pickle.dumps(response) | ||
| unpickled_response = pickle.loads(pickled_response) | ||
| assert unpickled_response == response |
There was a problem hiding this comment.
Seems like we could have a function that specifically takes Any and does this pickle roundtripping. Not sure if we'll ever add more logic to this, but having it in one place would make that easier.
There was a problem hiding this comment.
Sorta like in test/unit/test_quilbase.py
There was a problem hiding this comment.
I like that idea, I parametrized the response types over a single test implementation.
There was a problem hiding this comment.
This is great, and will be appreciated by many!
Do you know if pickling allow objects to be serialized/deserialized between Python runtimes on separate architectures? For example, if someone were to pickle execution results on ppc64le using pyquil-grpc-web would they then be able to unpickle(?) from arm64 pyquil?
Not a blocking question, just curious.
Co-authored-by: jselig-rigetti <[email protected]>
While that isn't something I have first-hand experience with, the pickling format should be platform-agnostic. There are versioned protocols, but none of them mention platform specificity as an issue. |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
| numpy = "^1.25" | ||
| scipy = "^1.11" | ||
| rpcq = "^3.11.0" | ||
| networkx = ">=2.5" | ||
| qcs-sdk-python = "0.19.0" | ||
| qcs-sdk-python = "0.19.2" | ||
| quil = ">=0.11.2" |
There was a problem hiding this comment.
Prior to qcs-sdk-python 0.19.2, quil was required to be a specific version. That was meant to ensure compatibility between downstream users of both packages, but in practice that has been over-restrictive. This allows quil to be updated separately from qcs-sdk-python, at least between breaking changes.
pyproject.toml
Outdated
| @@ -19,12 +19,13 @@ packages = [{ include = "pyquil" }] | |||
| exclude = ["pyquil/conftest.py"] | |||
|
|
|||
| [tool.poetry.dependencies] | |||
| python = "^3.9,<=3.14" | |||
| python = "^3.9,<=4.0" | |||
There was a problem hiding this comment.
This will close #1793.
test/unit/test_qpu.py
Outdated
| # result_data=ResultData.from_qvm( | ||
| # QVMResultData.from_memory_map({ | ||
| # "int": RegisterData.from_i8([[1, 2, 3], [4, 5, 6]]), | ||
| # "float": RegisterData.from_f64([[0.1, 0.2, 0.3], [0.4, 0.5, 0.6]]) | ||
| # }) | ||
| # ) |
test/unit/test_qpu.py
Outdated
| def test_pickle_qpu_execute_response(mock_encrypted_program): | ||
| response = QPUExecuteResponse( | ||
| job_id="some-job-id", | ||
| _executable=mock_encrypted_program, | ||
| execution_options=ExecutionOptions.default() | ||
| ) | ||
| pickled_response = pickle.dumps(response) | ||
| unpickled_response = pickle.loads(pickled_response) | ||
| assert unpickled_response == response |
There was a problem hiding this comment.
I like that idea, I parametrized the response types over a single test implementation.
Description
This closes #1719 (along w/ an additional request to support
QPUExecuteResponse), and #1791 (but for all instruction classes).Waiting on some upstream changes to land, once they do, this PR should pass CI.
Checklist
masterbranch