Rename UCCSD functions and remove isclose

[kevinsung]

PRs https://github.com/quantumlib/OpenFermion/pull/257 and https://github.com/quantumlib/OpenFermion/pull/267 renamed some functions and in this PR I port over these name changes.

There is one thing though: the changes made in those PRs break the tests UnitaryCC.test_simulation_energy and UnitaryCC.test_simulation_with_graph in openfermionprojectq/_unitary_cc_test.py. That needs to be fixed before the next version release. These tests are quite opaque to me; I don't know where these hard-coded values came from and it's hard for me to tell from reading the source code what exactly uccsd_trotter_engine does. Can somebody else take a look at this? I might be able to fix it with some help/advice/suggestions.

[kevinsung]

I don't think hard-coding values is generally a good way to write tests. I suggest another way to write this test: whatever uccsd_trotter_engine is supposed to do (I don't know), let's simulate it using sparse matrices (e.g., scipy.sparse.linalg.expm_multiply), and check that the results are the same. Does that make sense?

[jarrodmcc]

uccsd_trotter_engine wraps up the required boiler plate to make projectq spit out a circuit using 1- and 2-qubit unitaries, with an option to take a specific hardware graph.

The energies in test_simulation_energy come from an exact simulation of the classical coupled cluster energies from Psi4 and were meant to check a case where the two match exactly (2 electrons). We don't really have anything in the code base built to do this calculation at the moment, I suppose it's more of a convention check than anything on the input to singlet evolution. If the energy matches after changing the names to match, that is likely sufficient for now.

I'm unhappy in general with the complexity of unit testing certain parts of the projectQ compiler, which makes it very hard to individually test the functioning of things like the "gate decomposer" and "gate recognizer" which have opposite conventions. Hence the tests are all a bit wonky in this part.

It's also worth mentioning that we shouldn't spend a whole lot of time optimizing this part of the UCCSD->hardware translation, as I believe we will soon supersede this with a vastly superior theoretical compilation of UCCSD circuits.

[kevinsung]

The problem is that the energies don't match after changing names. I also would rather not spend time on this.

[jarrodmcc]

At a glance it looks like we don't actually test the energy of the singlet generator in utils in the core of openfermion, so it's possibly broken there also, at least in the sense that the convention has changed in a way that doesn't let us verify against the exact solution anymore. I won't be able to check this in detail today, but perhaps later this week

On Wed, Mar 21, 2018 at 10:25 AM, Kevin J. Sung notifications@github.com wrote:

The problem is that the energies don't match after changing names. I also would rather not spend time on this.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quantumlib/OpenFermion-ProjectQ/pull/42#issuecomment-375027533, or mute the thread https://github.com/notifications/unsubscribe-auth/AHXFPE4IEfxi926wdzoP_GFZ09rgcuVKks5tgo0lgaJpZM4SzC-p .

[kevinsung]

Ok I think I understand what you're saying about the conventions. How about for now, I get rid of these tests and replace them with a few simple ones that don't depend on conventions to get things working with the development version of OpenFermion, and we can worry about the energy thing later?

[jarrodmcc]

That seems okay as long as there exists a logical convention that gets the energy to match the exact H2 energy.

On Wed, Mar 21, 2018 at 10:56 AM, Kevin J. Sung notifications@github.com wrote:

Ok I think I understand what you're saying about the conventions. How about for now, I get rid of these tests and replace them with a few simple ones that don't depend on conventions to get things working with the development version of OpenFermion, and we can worry about the energy thing later?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quantumlib/OpenFermion-ProjectQ/pull/42#issuecomment-375038616, or mute the thread https://github.com/notifications/unsubscribe-auth/AHXFPNOVppMb25ASt8ATvI5swn4zBVDcks5tgpRGgaJpZM4SzC-p .

[kevinsung]

Now that I think of it, the convention issue probably has to do with how redundancies in the amplitudes were considered. The old UCCSD generator code looped over the spin and hence added some amplitudes twice, whereas the new one only adds them once. I'll look into that.

[kevinsung]

Ok I actually fixed the coefficients thing, no need to change the tests. I also made this PR fix the compatibility issue with isclose.

[babbush]

@kevinsung what is the status of this now? Is this ready to go once we make a new release of OpenFermion and bump the version requirement. You should probably go ahead and edit requirements.txt so that the first line reads openfermion>=0.5 since that is the version that will (presumably) fix things.

[kevinsung]

I just reran pytest with this branch and the development version of OpenFermion, and now _time_evolution_test.py is giving a bunch of errors... I'll look into this soon.

[kevinsung]

The failing tests were due to the fact that I changed some statements of the form A.isclose(B) to A == B where A is an instance of projectq.ops.QubitOperator and B is an instance of openfermion.ops.QubitOperator. The problem is that these are different types, so A == B should always evaluate to False. To remedy this I just left those tests in that form, A.isclose(B). This is fine, even when we eventually get ride of the deprecated isclose, as long as ProjectQ does not get rid of isclose. If they ever do that then we'd have to update these tests.

Anyway, this PR is ready to be merged once OpenFermion's version is updated. If for some reason it can't be, I'll make it my first priority to fix it.

[babbush]

Okay, in your opinion, are we ready for another version update? Or do you expect to introduce more breaking changes this week?

[kevinsung]

I plan to resolve https://github.com/quantumlib/OpenFermion/issues/271 tonight (DeprecationWarnings and all), and after that we can bump the version. Does that sound okay?

[babbush]

Yes, that sounds great. Would you mind also making sure that none of these changes break the other plugins for Psi4 or PySCF. Unfortunately we don't have continuous integration on those repos so you'd just need to run stuff like the demo and data generation scripts by hand.

[kevinsung]

Sure, I'll do that.