quantumlib / OpenFermion

The electronic structure package for quantum computers.
Apache License 2.0
1.51k stars 374 forks source link

Let SymbolicOperator coeff be sympy expressions #457

Closed joesink closed 4 years ago

joesink commented 6 years ago

It would be useful to add sympy symbolic expression compatibility to the FermionicOperator/other operators to allow for cases when the amplitude on the operators is unknown.

babbush commented 6 years ago

This is something we've discussed in the past. Indeed, it would be very interesting and would be very useful. It would make OpenFermion a tool for deriving expressions in very general contexts.

It would be great to have more discussion of what the implementation might look like. I think we'd need to implement in such a fashion that the functionality is entirely optional. Otherwise having symbolic objects everywhere would probably kill performance for some important applications.

obriente commented 4 years ago

I think I'll need this in my course, as it will allow for better integration with cirq. I'd like to show UCC by saying 'we write the cluster operator in OpenFermion with free parameters and then exponentiate and Trotterize', rather than having to introduce a nice package and then having to do something that feels quite natural by hand. So I'd like to take this on.

I propose beginning by removing the type-checking on SymbolicOperator's coefficient, and then writing some tests to check that algebraic operations and conversion between the different types of SymbolicOperators works as expected (and patching when necessary). This has the advantage that it doesn't reduce performance for anyone who isn't using sympy. It has the disadvantage that we're letting more errors slip, but I think initiating an operator with an inappropriate coefficient is something that will be easily traceable --- please let me know if you think otherwise.

I think that we can't expect to support sympy in every piece of code in OpenFermion --- I would suggest that anything that goes beyond the standard algebraic operations go unsupported (at least for now), and we be explicit about this.

Thoughts?

bryano commented 4 years ago

OFC#353 is relevant; it's intended to be a first step towards 'writing the cluster operator with free parameters and then exponentiating and trotterizing'. Once that and #549 are in, I can submit a PR with my CoupledCluster class in OFC.

babbush commented 4 years ago

@ncrubin was thinking about doing something related recently. I worry that this could become very unwieldy if we're not careful with it. Tom can you explain the use case a bit more so we get a better picture of what you're hoping to do?

ncrubin commented 4 years ago

I think having symbolic expressions for SymbolicOperator and any derived class is an interesting idea and potentially a huge undertaking. One thing to consider is how basic algebraic operations will work and mesh with symbolic operations. I believe symbols in sympy build up an object tree and then evaluate when some function is called requiring the value of the symbol. But how will eq work in derived classes from SymbolicOperator? Maybe the same way? Just something to consider. We might need to reimplement equality. I think multiplication, division, and subtraction should be fine but a pilot implementation might be prudent. One of my fears is that right now it is easy to recycle FermionOperators but with sympy expressions it would be carrying around another object (the sympy symbol and expression tree) that the user would need to be aware of.

Using sympy might be sufficient to take care of all these issues. Just something to check with a sample implementation or research into how Sympy.Exp works. I raise these issues having built a terrible symbolic expression evaluation in the past which caused more issues that in worth....but the past is the past...maybe this could be great.

obriente commented 4 years ago

@bryano - indeed; I had noticed that PR and was very happy that you'd worked this out. Is there anything I can help with there? (Also, I might want to ask you how to put this all together.)

@ncrubin - Thanks for the warnings, they're definitely worth paying attention to. For equality testing, what I was planning was to check whether 'abs(coeff1-coeff2)<EQ_TOLERANCE' for each pair of coefficients. This seems to work in simple cases, at least, though using sympy.simplify appears to be critical if you want to guarantee things cancel. I've tried to not use of sympy.simplify in openfermion itself for now, as this appears to be quite resource-intense (and so I feel leaving this to the user is the best way to guarantee performance).

@babbush - So the use case I was thinking of was creating the cluster operator in a 'openfermionic' method to go with openfermion-cirq. This looks like it will be much better done using the PR that @bryano mentioned above, but as an example using some code from openfermion.utils._trotter_exp_to_qgates


from openfermion import FermionOperator, jordan_wigner, trotterize_exp_qubop_to_qasm, hermitian_conjugated
import sympy
num_filled_orbitals = 3
num_empty_orbitals = 2
cluster_operator = FermionOperator()
parameter_list = []
for i in range(num_filled_orbitals):
   for j in range(i+1, num_filled_orbitals):
        for k in range(num_empty_orbitals:
            for l in range(k+1, num_empty_orbitals):
                theta = sympy.Symbol('theta'+str(i)+str(j)+str(k)+str(l))
                cluster_operator += FermionOperator(str(i)+'^ '+str(j)+'^ '+str(k)+' '+str(l)+' ', theta)
                parameter_list.append(theta)
cluster_operator_jw_ah = jordan_wigner(cluster_operator - hermitian_conjugated(cluster_operator))
# Might want to make an openfermion command to do this directly
for term in cluster_operator_jw_ah.terms:
    cluster_operator_jw_ah[term] = sympy.simplify(cluster_operator_jw_ah[term])
qasm = trotterize_exp_qubop_to_qasm(cluster_operator_jw_ah)
bryano commented 4 years ago

My trotterization code takes in an InteractionOperator rather than a SymbolicOperator as in #549, so I

  1. defined a SymbolicTensor class that just adds convenience methods to a defaultdict, and
  2. defined a SymbolicInteractionOperator whose tensors are SymbolicTensors, so that it quacks like an InteractionOperator.

The result is a bit hacky, but it works. For UCC, each coefficient is a simple multiple of a single Symbol, which is something we can enforce.

obriente commented 4 years ago

Hmmm, the idea of defining a whole new class for sympy expressions might be a way to do this. Perhaps we can use a sympy.Array to store the data? This might be the best way to utilize the tensor structure.

Out of curiosity, what made you choose InteractionOperators instead of FermionOperators for the trotterization function? To me the latter are in a more quantum-computing-friendly format.

bryano commented 4 years ago

For non-symbolic instances, InteractionOperator seemed much more efficient, and I was only interested in chemically-relevant Hamiltonians, which are fully captured by InteractionOperator.

obriente commented 4 years ago

Fair enough, that makes sense.

What work would need to be added to your code to allow it to interface with FermionOperators directly (without going through the InteractionOperator representation)? I'd be happy to both write that up and take a look at your SymbolicInteractionOperator class (you mentioned above that you've already written this code? --- if not I can also give it a stab).

bryano commented 4 years ago

In OFC#353, fermionic_simulation_gates_from_interaction_operator takes in an InteractionOperator and returns the gates (as a mapping from fermionic modes to the gate that acts on them) corresponding to a trotterization of the exponential of the interaction operator using Jordan-Wigner. This in turn delegates to the from_interaction method of FermionicSimulationGate.

Extending the code to FermionOperator would entail simply implementing something analogous. For testing purposes, there's also interaction_operator_from_fermionic_simulation_gates, which does the inverse, and something like this should probably also be implemented for FermionOperator.

This is all in openfermioncirq.gates.fermionic_simulation.

mstechly commented 4 years ago

Just wanted to say that it would be great to have this feature added to OpenFermion – can't wait to see #549 merged in :)

ncrubin commented 4 years ago

549 was merged!