Keep up with quimb-1.5.0 #6087
Keep up with quimb-1.5.0 #6087
Conversation
As of quimb-1.5.0 `TensorNetwork.contract(inplace=True)` returns TensorNetwork instead of scalar. Convert to scalar if needed.
Requires next cirq release.
The test should actually pass in `dev_tools/notebooks/notebook_test.py` which tests with the head version of Cirq. This reverts commit 3817f54.
When `dev_tools/notebooks/notebook_test.py` is run from the CI job `Notebook Tests against PR` it is executed with check/pytest which adjusts PYTHONPATH to use the work-tree sources of Cirq. Rename `test_notebooks_against_released_cirq` --> `test_notebooks_against_cirq_head`. Note: Notebooks are tested with released Cirq in isolated_notebook_test.py.
The `test_notebooks_against_cirq_head` uses a temporary directory for notebook installed packages. Put this directory as last to PYTHONPATH so when some notebooks pip-install Cirq, the working sources of Cirq are still loaded first. This fixes the test of `Contract-a-Grid-Circuit.ipynb` which needs an unreleased Cirq update.
Requires next cirq release.
|
@vtomole - actually the original CI failure was for a notebook tested with cirq head so it should have the fix in this PR. The isolated_notebook_test.py tests w/r to the released cirq so I have disabled the affected notebook there. PS: can you PTAL again? |
| @@ -177,6 +177,8 @@ def tensor_expectation_value( | |||
| if ram_gb > max_ram_gb: | |||
| raise MemoryError(f"We estimate that this contraction will take too much RAM! {ram_gb} GB") | |||
| e_val = tn.contract(inplace=True) | |||
| if isinstance(e_val, qtn.TensorNetwork): | |||
| e_val = e_val.item() | |||
There was a problem hiding this comment.
AFAICT, the contraction above will always produce a scalar, so why not call item unconditionally?
e_val = tn.contract(inplace=True).item()There was a problem hiding this comment.
With quimb-1.4.2 tn.contract() may return a standard float which does not have the item method.
We could do e_val = np.asarray(tn.contract(...)).item(), but that is IMO harder to parse for humans than above. :)
We could also require quimb==1.5, but I feel that is a bit imposing on anyone installing cirq.
dev_tools/notebooks/notebook_test.py
Outdated
| @@ -73,8 +73,11 @@ def require_packages_not_changed(): | |||
| def env_with_temporary_pip_target(): | |||
| """Setup system environment that tells pip to install packages to a temporary directory.""" | |||
| with tempfile.TemporaryDirectory(suffix='-notebook-site-packages') as tmpdirname: | |||
| # Note: We need to append the PYTHONPATH here so that development Cirq, | |||
There was a problem hiding this comment.
Ack. This test is run with check/pytest which sets PYTHONPATH to use the work-tree sources.
The line below appends tmpdirname to the PYTHONPATH.
When some notebook executes pip install cirq, that stable version would appear in PYTHONPATH after the worktree sources (which we want to use in this test).
Let me reword for clarity.
Cirq/dev_tools/notebooks/notebook_test.py
Lines 76 to 83 in cc765a8
As of quimb-1.5.0 `TensorNetwork.contract(inplace=True)` returns TensorNetwork instead of scalar. Convert to scalar if needed. Also correct CI issues that showed for testing notebooks w/r to the cirq head here: * Fix misleading unit test name by renaming `test_notebooks_against_released_cirq` --> `test_notebooks_against_cirq_head`. Note: Notebooks are tested with released Cirq in isolated_notebook_test.py. * Append to PYTHONPATH when tested notebooks pip-install any packages. Prefer use of development Cirq sources in `test_notebooks_against_cirq_head`. * Disable isolated CI test of Contract-a-Grid-Circuit.ipynb Requires next cirq release.
As of quimb-1.5.0
TensorNetwork.contract(inplace=True)returnsTensorNetwork instead of scalar. Convert to scalar if needed.