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

Add walk operator example for sparse chem ham. #882

Merged
merged 30 commits into from
May 10, 2024

Conversation

fdmalone
Copy link
Collaborator

@fdmalone fdmalone commented Apr 17, 2024

Working on #841, adds a walk operator example for the sparse hamiltonian which fits the usual select and prepare construction. Just adds an example and some autodoc. I have an organizational / doc question: I added a BloqExample for building the walk operator BUT this reproduces the docstring for QubitizationWalkOperator in sparse.ipynb which isn't ideal. Would it be better to add this as an example in the notebook for QubitizationWalkOperator, I feel this might make the notebook a bit crowded if we keep adding examples there?

Currently there is a test failing for serialization which I don't really understand yet because Prepare and Select pass the serialization test? Any idea @tanujkhattar?

I also fixed the latex for QubitizationWalkOperator.

@fdmalone
Copy link
Collaborator Author

Serialization error:

self = PrepareSparse(num_spin_orb=8, num_non_zero=65, num_bits_state_prep=12, num_bits_rot_aa=8, adjoint=False, qroam_block_size=None)
other = PrepareSparse(num_spin_orb=8, num_non_zero=65, num_bits_state_prep=12, num_bits_rot_aa=8, adjoint=0, qroam_block_size=None)

    def __eq__(self, other):
        if other.__class__ is not self.__class__:
            return NotImplemented
>       return  (
            self.num_spin_orb,
            self.num_non_zero,
            self.num_bits_state_prep,
            self.alt_pqrs,
            self.alt_theta,
            self.alt_one_body,
            self.ind_pqrs,
            self.theta,
            self.one_body,
            self.keep,
            self.num_bits_rot_aa,
            self.adjoint,
            self.qroam_block_size,
        ) == (
            other.num_spin_orb,
            other.num_non_zero,
            other.num_bits_state_prep,
            other.alt_pqrs,
            other.alt_theta,
            other.alt_one_body,
            other.ind_pqrs,
            other.theta,
            other.one_body,
            other.keep,
            other.num_bits_rot_aa,
            other.adjoint,
            other.qroam_block_size,
        )
E       ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

@fdmalone
Copy link
Collaborator Author

And for QPE:

qualtran/bloqs/phase_estimation/qubitization_qpe_test.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
qualtran/conftest.py:103: in assert_bloq_example_typing_for_pytest
    qlt_testing.assert_bloq_example_preserves_types(bloq_ex)
qualtran/testing.py:587: in assert_bloq_example_preserves_types
    cbloq = bloq.decompose_bloq()
qualtran/_infra/gate_with_registers.py:245: in decompose_bloq
    return decompose_from_cirq_style_method(self)
qualtran/cirq_interop/_cirq_to_bloq.py:574: in decompose_from_cirq_style_method
    raise exc
qualtran/cirq_interop/_cirq_to_bloq.py:567: in decompose_from_cirq_style_method
    return cirq_optree_to_cbloq(
qualtran/cirq_interop/_cirq_to_bloq.py:444: in cirq_optree_to_cbloq
    circuit = cirq.Circuit(optree)
../../.virtualenvs/qualtran/lib/python3.11/site-packages/cirq/circuits/circuit.py:1771: in __init__
    flattened_contents = tuple(ops.flatten_to_ops_or_moments(contents))
../../.virtualenvs/qualtran/lib/python3.11/site-packages/cirq/ops/op_tree.py:134: in flatten_to_ops_or_moments
    yield from flatten_to_ops_or_moments(subtree)
../../.virtualenvs/qualtran/lib/python3.11/site-packages/cirq/ops/op_tree.py:133: in flatten_to_ops_or_moments
    for subtree in root:
qualtran/bloqs/phase_estimation/qubitization_qpe.py:137: in decompose_from_registers
    walk_controlled = self.walk.controlled(control_values=[1])
qualtran/bloqs/qubitization_walk_operator.py:135: in controlled
    c_select = self.select.controlled(control_values=control_values)
qualtran/_infra/gate_with_registers.py:323: in controlled
    controlled_gate = cirq.ControlledGate(
../../.virtualenvs/qualtran/lib/python3.11/site-packages/cirq/ops/controlled_gate.py:88: in __init__
    _validate_sub_object(sub_gate)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

sub_object = SelectSparse(num_spin_orb=8, control_val=None)

    def _validate_sub_object(sub_object: Union['cirq.Gate', 'cirq.Operation']):
        if protocols.is_measurement(sub_object):
            raise ValueError(f'Cannot control measurement {sub_object}')
        if not protocols.has_mixture(sub_object) and not protocols.is_parameterized(sub_object):
>           raise ValueError(f'Cannot control channel with non-unitary operators: {sub_object}')
E           ValueError: Cannot control channel with non-unitary operators: SelectSparse(num_spin_orb=8, control_val=None)

../../.virtualenvs/qualtran/lib/python3.11/site-packages/cirq/ops/controlled_gate.py:357: ValueError

@fdmalone
Copy link
Collaborator Author

Ok, I guess walk_operator isn't serializable right now, I updated the set of bloqs to skip. Just the control error now.

@tanujkhattar
Copy link
Collaborator

I think the control error should be fixed once my inflight PR #879 is merged.

@tanujkhattar
Copy link
Collaborator

tanujkhattar commented Apr 17, 2024

def __eq__(self, other):

I think this is happening because one of the elements in the tuple is a numpy array (either before or after serialization / deserialization roundtrip). If any of this is a attrs attribute and expected to be a Sequence, we should add a default attrs.field(converter=tuple) to expect any sequence and convert it to a tuple.

The error seems to be not in serialization / deserialization roundtrip but rather when comparing the roundtrip object with original object. If the serialization roundtrip is messing with the attributes, it'd be good to open a new issue but my guess is that it's serialization; for example; a list of ints/floats to a numpy array of ints/floats and deserializing back to numpy array which is messing with equality because we don't have a default converter.

cc @Epsilon1024 Maybe you also have thoughts here.

@fdmalone
Copy link
Collaborator Author

fdmalone commented Apr 17, 2024

It's odd because Prepare (which it's complaining about) does correctly serialize without issue when it's not apart of the walk operator, I noticed that the example walk_op was being skipped already. I can't remember why serialization for walk op is skipped.

@fdmalone
Copy link
Collaborator Author

Screenshot 2024-04-17 at 9 25 42 PM

@fdmalone
Copy link
Collaborator Author

fdmalone commented Apr 24, 2024

@tanujkhattar PTAL, I fixed a few bugs a long the way:

  1. There was an extra comparator which shouldn't have been there. Prepare^ will uncompute this.
  2. There was a missing single-qubit CSWAP.
  3. I removed the special case for adjoint in the call graph so the decomposition matches the call graph, but this brings us farther away from the reference value because of QROM / inequality / CSWAP inversion cost discrepancies.
  4. I fixed a sign error in the cost for uniform state preparation.
  5. Using SelectSwapQROM makes the tests a bit slower so I reduced the system sizes in sparse_test.py.

The qubitization_qpe serialization test for chemistry was too slow, so I also just add this to the skipped tests.

@fdmalone fdmalone requested a review from mpharrigan May 6, 2024 20:50
@fdmalone fdmalone enabled auto-merge (squash) May 10, 2024 16:36
@fdmalone fdmalone merged commit 49fe6b8 into quantumlib:main May 10, 2024
6 of 7 checks passed
@fdmalone
Copy link
Collaborator Author

That was weird, them mypy check failed but it automerged? @mpharrigan

@mpharrigan
Copy link
Collaborator

#926 configured the CI job but someone with github admin (hopefully me) needs to set it as a required check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants