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

Cirq inverse seems to (at least sometimes) modify the values given to it, is this intended? #1087

Closed
Wuggins opened this issue Oct 31, 2018 · 3 comments

Comments

@Wuggins
Copy link

Wuggins commented Oct 31, 2018

First of all, I apologize if this should have been posted to Stack Exchange instead. It seemed specific enough to the code that I thought I would post it here.

It seems that, at least for some inputs, cirq.inverse(val) modifies the input that it is given. Is this the intended behavior? It seems counterintuitive from a user perspective that

circuit.append(cirq.inverse(val))
circuit.append(val)

would yield anything other than an effective identity operation.

This behavior can be seen specifically in the example notebook provided by OpenFermion-Cirq here:
https://github.com/quantumlib/OpenFermion-Cirq/blob/master/examples/tutorial_1_basis_change.ipynb

In particular, one can modify the second cell in the notebook in the following way:

import openfermioncirq
import cirq

# Initialize the qubit register.
qubits = cirq.LineQubit.range(n_qubits)

# Start circuit with the inverse basis rotation, print out this step.
basis_rotation = openfermioncirq.bogoliubov_transform(qubits, basis_transformation_matrix)
inverse_basis_rotation = cirq.inverse(basis_rotation)
#inverse_basis_rotation = cirq.inverse(openfermioncirq.bogoliubov_transform(qubits, basis_transformation_matrix))
circuit = cirq.Circuit.from_ops(inverse_basis_rotation)
print(circuit)

# Add diagonal phase rotations to circuit.
for k, eigenvalue in enumerate(eigenvalues):
    phase = -eigenvalue * simulation_time
    circuit.append(cirq.Rz(rads=phase).on(qubits[k]))

# Finally, restore basis.
#basis_rotation = openfermioncirq.bogoliubov_transform(qubits, basis_transformation_matrix)
circuit.append(basis_rotation)

to yield something which no longer acts as expected.

@kevinsung
Copy link
Collaborator

kevinsung commented Oct 31, 2018

Indeed, cirq.inverse will modify an iterator by "using it up", i.e., calling its __next__() method repeatedly. In this example, basis_rotation is a generator iterator returned by the function bogoliubov_transform. That's the reason for the repeated calls to bogoliubov_transform. One could avoid this behavior by converting the iterator to a tuple or list.

This is intended behavior; using iterators to generate operations is a common pattern in Cirq.

@Strilanc
Copy link
Contributor

Strilanc commented Oct 31, 2018

This is why I've been moving away from exposing methods returning OP_TREE to users, and always flattening them into lists first. We should do the same thing in openfermion-cirq. I've opened quantumlib/OpenFermion-Cirq#302 there, so that for example this mistake wouldn't happen.

In other words, I do think that this is a bug but I think it's a usability bug in openfermioncirq.bogoliubov_transform as opposed to a behavior bug in cirq.inverse.

@Wuggins
Copy link
Author

Wuggins commented Nov 2, 2018

Thanks for the explanation! It certainly wasn't the expected behavior but I'm coming to understand the way that iterators are used throughout the codebase as I get deeper into it.

Thanks,

Bill

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

No branches or pull requests

3 participants