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

Forging is not yet compatible with Qiskit Nature 0.7 and higher #406

Closed
garrison opened this issue Sep 7, 2023 · 3 comments
Closed

Forging is not yet compatible with Qiskit Nature 0.7 and higher #406

garrison opened this issue Sep 7, 2023 · 3 comments
Labels
cicd Related to the CICD pipeline urgent
Milestone

Comments

@garrison
Copy link
Member

garrison commented Sep 7, 2023

I haven't figured out exactly why yet, but the merging of qiskit-community/qiskit-nature#1248 broke our "development version tests."

=================================== FAILURES ===================================
_ TestEntanglementForgingGroundStateSolver.test_entanglement_forging_vqe_hydrogen _

self = <test.forging.test_entanglement_forging_ground_state_solver.TestEntanglementForgingGroundStateSolver testMethod=test_entanglement_forging_vqe_hydrogen>

    @unittest.skipIf(not pyscf_available, "pyscf is not installed")
    def test_entanglement_forging_vqe_hydrogen(self):
        """Test of applying Entanglement Forged Solver to compute the energy of a H2 molecule."""
        # Set up the ElectronicStructureProblem
        driver = PySCFDriver(
            atom="H .0 .0 .0; H .0 .0 0.735",
            unit=DistanceUnit.ANGSTROM,
            charge=0,
            spin=0,
            basis="sto3g",
        )
        driver.run()
        problem = driver.to_problem(basis=ElectronicBasis.AO)
        qcschema = driver.to_qcschema()
        mo_coeff = get_ao_to_mo_from_qcschema(qcschema).coefficients.alpha["+-"]
    
        # Specify the ansatz and bitstrings
        ansatz = EntanglementForgingAnsatz(
            circuit_u=TwoLocal(2, [], "cry", [[0, 1], [1, 0]], reps=1),
            bitstrings_u=[(1, 0), (0, 1)],
        )
    
        # Set up the entanglement forging vqe object
        solver = EntanglementForgingGroundStateSolver(
            ansatz=ansatz,
            optimizer=SPSA(maxiter=0),
            initial_point=[0.0, np.pi / 2],
            mo_coeff=mo_coeff,
        )
    
        # Solve for the ground state energy
>       results = solver.solve(problem)

test/forging/test_entanglement_forging_ground_state_solver.py:125: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
circuit_knitting/forging/entanglement_forging_ground_state_solver.py:299: in solve
    hamiltonian_terms = self.get_qubit_operators(problem)
circuit_knitting/forging/entanglement_forging_ground_state_solver.py:399: in get_qubit_operators
    hamiltonian_ops, self._energy_shift = cholesky_decomposition(
circuit_knitting/forging/cholesky_decomposition.py:110: in cholesky_decomposition
    h_1_op, h_chol_ops, freeze_shift, _, _ = _get_fermionic_ops_with_cholesky(
circuit_knitting/forging/cholesky_decomposition.py:266: in _get_fermionic_ops_with_cholesky
    coeff_mo = copy.copy(mo_coeff)
../../miniconda3/lib/python3.9/copy.py:102: in copy
    return _reconstruct(x, None, *rv)
../../miniconda3/lib/python3.9/copy.py:271: in _reconstruct
    if hasattr(y, '__setstate__'):
.tox/py/lib/python3.9/site-packages/qiskit_nature/second_q/operators/tensor.py:194: in __getattr__
    array = self.__array__()
.tox/py/lib/python3.9/site-packages/qiskit_nature/second_q/operators/tensor.py:145: in __array__
    return self._array
.tox/py/lib/python3.9/site-packages/qiskit_nature/second_q/operators/tensor.py:194: in __getattr__
    array = self.__array__()
.tox/py/lib/python3.9/site-packages/qiskit_nature/second_q/operators/tensor.py:145: in __array__
    return self._array
E   RecursionError: maximum recursion depth exceeded
!!! Recursion detected (same locals & position)
@garrison garrison added the cicd Related to the CICD pipeline label Sep 7, 2023
@garrison garrison changed the title Development version tests broken Development version tests broken due to change in Qiskit Nature Sep 7, 2023
@garrison
Copy link
Member Author

garrison commented Sep 7, 2023

Specifically, the changed behavior relates to the following line:

https://github.com/Qiskit-Extensions/circuit-knitting-toolbox/blob/8c23c30e0ee4c2bb81e3766ade07ea1dc0ebb98c/test/forging/test_entanglement_forging_ground_state_solver.py#L108

Before qiskit-community/qiskit-nature#1248 landed, this line resulted in mo_coeff being an ndarray. But now, with the most recent Qiskit Nature main, it is instead a qiskit_nature.second_q.operators.tensor.Tensor.

@garrison
Copy link
Member Author

garrison commented Sep 7, 2023

We should also revert #407 as part of the fix to this, when that day comes.

garrison added a commit that referenced this issue Sep 7, 2023
@garrison garrison added this to the 0.4.1 milestone Sep 13, 2023
@garrison garrison modified the milestones: 0.4.1, 0.5.1 Oct 31, 2023
@garrison garrison changed the title Development version tests broken due to change in Qiskit Nature Not yet compatible with Qiskit Nature 0.7 and higher Nov 9, 2023
@garrison garrison changed the title Not yet compatible with Qiskit Nature 0.7 and higher Forging is not yet compatible with Qiskit Nature 0.7 and higher Nov 9, 2023
@garrison
Copy link
Member Author

garrison commented Nov 12, 2023

I've added the "urgent" label, because now that Qiskit/qiskit#11086 (removal of qiskit.algorithms) has been merged to Qiskit, CKT no longer works with Qiskit main, which is expected to become Qiskit 1.0. The problem is that we are now pinned to qiskit-nature>=0.6.0, < 0.7, but the latest Qiskit Nature consistent with that constraint still imports -- in various places -- from qiskit.algorithms and nowhere from qiskit_algorithms.

One thing we could ask for is a qiskit-nature 0.6.3 release that depends on and imports from qiskit_algorithms instead of qiskit.algorithms, so that code relying on Qiskit Nature 0.6.X can continue to work. Another option is to make our code compatible with Qiskit Nature 0.7 (this issue). And, of course, another option is to continue working toward removing the Qiskit Nature dependency altogether (#275). No matter what we choose, it looks like Jan 31 is our deadline if we want to continue being compatible with the latest Qiskit release (and we have much less time if we want to be compatible with Qiskit 1.0.0pre1).

caleb-johnson added a commit that referenced this issue Feb 12, 2024
* Unwrap the `Tensor` objects from Qiskit Nature as necessary

Fixes #406

* Revert "Temporarily remove Qiskit Nature from dev version tests (#407)"

This reverts commit 88e2195.

* `unfold` the `S8Integrals`

* Install qiskit only, not qiskit-terra

See Qiskit/qiskit#11271.

* Unpin qiskit-nature

We'll probably also need to bump the version to 0.7 or higher,
but let's do that once the code is ready.

* Update forging to support Qiskit 1.0 and Nature 0.7

* Remove serverless tutorials

* Update ruff config

* Ignore forging tutorial 1

* Update qiskit readme shield. Update dev version of Qiskit to point to main branch

* Move ruff changes out of this PR

* Remove 0.46 from dev version testing

* Update path to qiskit TwoQubitWeylDecomposition

* Use 0.46 targets until 1.0 is released.

* Get all tests passing by removing problematic test modules

* ruff

* Bump rustwork version to be compatible w Qiskit 1.0

* Bump dev version to 1.0 from 0.46

* Bug in dev workflow

* Remove do_while from PassManager used to remove resets

* Fix notebooks

* Put do_while back

* Fix recursion error with forging

* Dont ignore forging tutorial

* Install Nature from pypi

* Make compatible with 0.46+1.0

* .github/workflows/test_development_versions.yml

* Bug in cutting_experiments

* Use DoWhileController

* Fix impmort of DWController

* Bump min qiskit version

* bump max vers

---------

Co-authored-by: Jim Garrison <garrison@ibm.com>
Co-authored-by: Jim Garrison <jim@garrison.cc>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cicd Related to the CICD pipeline urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants