4/4! - transform functions. #232

conta877 · 2018-03-02T20:50:21Z

final pull request for #154
transforms demo,
predefined transforms & their tests

I really wanted to name the file code_transformers... 🤖

…d the decoder shifter accordinly

…ing mathematica code. it was giving wrong operators before, now the operators are fine but signs are wonky

…to count qubit/fermion labels from 1 instead of 0

…ht) as a tool to append the same code intance several times

…sts for symbolicBinary. minor mods for python3 comp.

…ode, added an error if qubit index in a code is out of bounds

babbush · 2018-03-03T17:18:20Z

src/openfermion/transforms/_code_transform_functions.py

+
+
+def _encoder_checksum(modes):
+    """ Outputs the encoder matrix for checksum codes.


Can you add a couple lines describing the checksum code?

babbush

Just add a little more explanation about some of these codes and we're good to go. Thanks for a great contribution overall!

babbush · 2018-03-03T17:22:00Z

src/openfermion/transforms/_code_transform_functions.py

+
+def weight_one_binary_addressing_code(exponent):
+    """ Weight-1 binary addressing code.
+


Again, it would be good to have some more explanation of what these codes are and why people might be interested in them.

kevinsung · 2018-03-03T17:50:41Z

src/openfermion/transforms/_code_transform_functions_test.py

+    def test_jordan_wigner(self):
+        hamiltonian, gs_energy = lih_hamiltonian()
+        code = jordan_wigner_code(4)
+        qubit_hamiltonian = binary_code_transform(hamiltonian, code)


Is this supposed to do the same thing as the already-implemented jordan_wigner? If so, can we add a test that it does? Same for Bravyi-Kitaev.

@conta877 @kevinsung @babbush I wonder if it would make sense to have jordan_wigner alias to this (eliminating duplicate code) - does being more general mean that this code is slower / is there any other reason not to?

I agree with @kevinsung 's suggestion but am very opposed to @idk3 's suggestion. The implementation of Jordan-Wigner using the CodeOperator is significantly more complex. The Jordan-Wigner code might be useful to forks of OpenFermion that don't involve CodeOperator. The performance is also likely better in the more simple implementation.

I agree that we should keep both. I also think its a good example of code transforms for people. I didn't check jordan-wigner implementation but the Bravyi-Kitaevs produce different qubitOperators.

Really ? Let me have a look at that - I tend to accidentally switch the two matrices. In any case it seems to be a valid transform, since it passed the test.

Because the mode with the highest index is always the root of the Fenwick tree (see arXiv:1701.07072).

Implementing our version, I followed the instructions in 1208.5986 and S. Tranter's paper. There they segment the matrix if its in between two dimension that are consecutive integer powers of two. It seems like the Fenwick-tree mapping is different.

@msteudtner I think you're right. Perhaps this PR should use our implementation of Fenwick trees (contained in transforms) to define the Bravyi-Kitaev code? @babbush do you have an opinion? It seems that the code definitions are fundamentally different between 1208.5986 and 1701.07072.

Ok I just saw your comment below.

Based on the discussion below, I'm content with just checking that the JW (not BK) gives the same QubitOperators as the existing implementation, and that the new BK gives isospectral operators (does not need to give identical QubitOperators).

conta877 · 2018-03-03T19:27:34Z

what I noticed was the non code implementation (current openfermion version) generates extra factors for terms.(different than 1208.5986.) spectrums are the same.

…

On Mar 3, 2018 11:22, "Ryan Babbush" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/openfermion/transforms/_code_transform_functions_test.py <#232 (comment)> : > + ground_state_energy = eigenspectrum(hamiltonian)[0] + return hamiltonian, ground_state_energy + + +class CodeTransformTest(unittest.TestCase): + def test_checksum_code(self): + hamiltonian, gs_energy = lih_hamiltonian() + code = checksum_code(4, 0) + qubit_hamiltonian = binary_code_transform(hamiltonian, code) + self.assertAlmostEqual(gs_energy, + eigenspectrum(qubit_hamiltonian)[0]) + + def test_jordan_wigner(self): + hamiltonian, gs_energy = lih_hamiltonian() + code = jordan_wigner_code(4) + qubit_hamiltonian = binary_code_transform(hamiltonian, code) A good test would be to see if the transformations are all isospectral. How different is the output of the transformation? If there is a broad family of Bravyi-Kitaev like transformations with truly different structures it would be quite interesting. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#232 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AV-mvK7_12iWKNmYYBnJ03XGxHxVzuqsks5tau1vgaJpZM4SaexC> .

kevinsung · 2018-03-03T19:55:37Z

Is it true that arXiv:1208.5986 implicitly assumes that the number of qubits is a power of 2? Our implementation does not.

babbush · 2018-03-03T19:58:38Z

@kevinsung no, that is not assumed in any way by arXiv:1208.5986.

babbush · 2018-03-03T20:09:59Z

I had been under the impression that the Fenwick tree transform was just a different way of implementing essentially the same mapping. But you seem to be correct that there are real difference. This is interesting and suggests that perhaps we should have both types of BK implemented. I am curious to understand more about these differences. If I recall correctly, @VojtaHavlicek implemented the Fenwick tree BK. But @kanavsetia or @jarrodmcc may also have some insights about this difference...

In any event, this is really an issue that is unrelated to the quality of the pull request under review here. I think if you show your BK is isospectral and your JW gives the same JW transform as elsewhere in OpenFermion, that should be enough.

kevinsung · 2018-03-03T20:23:47Z

I have not scrutinized 1208.5986, but my understanding is that it defines the BK transform for powers of 2 and then reduces the case of a general integer to the power-of-2 case by using its binary expansion and applying multiple power-of-2 BK transforms.

kanavsetia · 2018-03-03T21:07:56Z

I think @kevinsung got it right. BK transform of 1208.5986 explicitly defines the transform for powers of 2 (as is evident from figure2 in the paper) but that is not the case for 1701.07072, which is more general.
And I think this is what is causing the discrepancy. For 1208.5986 no matter how many qubits you use, even qubits will always hold just their occupancy, and odd qubits contain their occupancy as well as occupancy of other adjacent qubits. So, if you are looking for the occupancy of 6th qubit, no matter the total number of qubits (which in this case will always be powers of 2), you will always just operate of 6th qubit.
But this is little different for 1701.07072, where, which qubit stores what is also dependent on the number of qubits. If you have total number of qubits as some power of 2, I think you will get the same answer as in 1208.5986. But for general case this will be different.

babbush · 2018-03-03T21:09:34Z

I understand what you are talking about now. I thought Kevin was saying it would only work for a binary power number of qubits.

kevinsung · 2018-03-03T21:32:53Z

Yes, the Fenwick tree approach is more general, and I think it should be relatively straightforward to use the existing Fenwick tree code to define the BinaryCode used in bravyi_kitaev_code (I admit I don't know the details though).

babbush · 2018-03-03T21:35:35Z

This would be a nice but I won't require it. So long as their Jordan-Wigner encoder matches our Jordan-Wigner and their Bravyi-Kitaev encoder gives the transformation from 1208.5986 while also being isospectral, to our JW/BK, I think that is good enough for now.

kevinsung · 2018-03-03T21:36:45Z

Sure, I agree that this should not be a requirement for this PR. However, if we plan to keep two different implementations of BK in OpenFermion then we should remember to make it clear in the documentation.

conta877 · 2018-03-03T22:17:27Z

Ok - I will add the tests and in the description for BK I'll mention which paper the transformation is based on.

conta877 · 2018-03-05T06:52:05Z

ready for review!

babbush

Just a few minor things. Thanks!

babbush · 2018-03-05T17:08:02Z

src/openfermion/transforms/__init__.py

@@ -16,6 +16,7 @@
 from ._bravyi_kitaev import bravyi_kitaev
 from ._binary_code_transform import (binary_code_transform,
                                     dissolve)
+from ._code_transform_functions import *


please write explicitly what you are importing here so that it is clear to everyone what exists in the namespace of the transforms module.

babbush · 2018-03-05T17:09:30Z

src/openfermion/transforms/_code_transform_functions.py

+    return mtx[0:dimension, 0:dimension]
+
+
+def _decoder_bk(modes):


I would recommend calling this n_modes to be consistent with the sort of notation we use elsewhere.

babbush · 2018-03-05T17:10:16Z

src/openfermion/transforms/_code_transform_functions.py

+from openfermion.ops import BinaryCode, SymbolicBinary, linearize_decoder
+
+
+def _encoder_bk(dimension):


There is probably some obvious reason why you use "dimension" here and "modes" below?

babbush · 2018-03-05T17:12:44Z

src/openfermion/transforms/_code_transform_functions.py

+    return BinaryCode(encoder, decoder)
+
+
+def weight_one_segment_code():


I'm a little confused why this function that only works for 2-qubits is necessary.

babbush · 2018-03-05T17:12:53Z

src/openfermion/transforms/_code_transform_functions.py

+                      ['w0 w1 + w0', 'w0 w1 + w1', ' w0 w1'])
+
+
+def weight_two_segment_code():


Same question as above.

conta877 · 2018-03-05T19:47:03Z

ready!

* fixed bohr/angstrom confusion in function description * translations from mathematica to python arxiv1712.07067 * work on those notimplementeds * WIP: decoder class adittion * modified decoder class. extractor implemented * no need for pauliaction function * introduction of symbolic binary class - no more decoder class * minor bugs fixed, applies binary rules to input now * started programming the BinaryCode class, work in progress * added count qubit function in binary operator, modified code.py * fixed morning brain bugs * implemented concatination and appending, not yet documented and debugged * added _shift function to symbolicBinary and integer addition. modified the decoder shifter accordinly * radd imul, NOT TESTED! - just copy paste should work? * fixed small bugs, added default codes, created transform * fixed the multiplication error and modified the transform code following mathematica code. it was giving wrong operators before, now the operators are fine but signs are wonky * fixed the transform and all the places where we accidentally started to count qubit/fermion labels from 1 instead of 0 * bug fixes: ordering matters to detect which terms should cancel * fixed code appending, introduced the integer multiplication (left+right) as a tool to append the same code intance several times * modified the multiply by 0 and 1 behavior in SymbolicBinary. added tests for symbolicBinary. minor mods for python3 comp. * fixed the condition for code concatenation, fixed a bug in checksum_code, added an error if qubit index in a code is out of bounds * added comments to binary_operator, more test, evaluate function * started writing documentations and clipping lines, writing out numpy * merging with the merger * updating binary operator based on comments * updating the binary_ops * updates based on comments * init update * moved binaryop to first import * added names into notice and readme * started documentation in _code_operator.py, added parity code / (K=1) and (K=2) segment code / K=1 binary addressing code * encoders are sparse matrices now * added tests for code_operator * doc strings for the transform, fixed bug in the initialization of SymbolicBinary, made changes suggested by review, carried some of them over to files to be pulled later * updates based on pull request comments * merging changes with merger * fixing possible integer checking errors * cleaned version of transform function, needs testing and timing - seems to work so far * fixed BK transforms. indexing errors * added tests, pep8 and Ryan compliance mods * added some style to the functions file * renaming, re-structuring * renaming again * minor changes * pep8 * changed the symbolicBInary class * docstring cleanup * bug docstring fixes * bug fix in the evaluate method, pep8 the _binary_operator file * merge with upstream and additional tests to SymbolicBinary class * added weight-two segment code test, fixed bug with in the code appending by integer multiplication * added half-up code * bugs fixed in half-up code * corrected weight_one_segment_code and the interleaved_code, former half-up. Added tests for the codes. * get rid of heads * reordering function for the fermionOperator class. takes 0,1,2,3,4,.. -> 0,2,4,..,1,3,... * spin indexing within moleculardata * started writing demo (work in progress), corrected typo in functions test , fixed possible bug for string parsing in symbolic binary file * changed function file test such that data is obtained half-up, added last part for ipython demo * corrected typos in the demo notebook * fixed bugs in test, it now runs without error * code transforms * pep8 * transform demo * fixes * typos fixing in demo * reverting back to original demo * demo testing & indent * docstring update * not ready for review! started on docs * descriptions for code functions * final pass at docs, fixing spaces * modes to n modes, and imports * fixed a bug in demo, added description to weight-n codes

conta877 and others added 30 commits January 18, 2018 09:36

fixed bohr/angstrom confusion in function description

5d9d368

Merge remote-tracking branch 'upstream/master'

ce92848

Merge remote-tracking branch 'upstream/master'

e45fe8b

translations from mathematica to python arxiv1712.07067

20d3e91

Merge remote-tracking branch 'upstream/master'

0fc95fb

work on those notimplementeds

91ab525

WIP: decoder class adittion

b8711fd

modified decoder class. extractor implemented

2ca9448

no need for pauliaction function

9036d0f

Merge remote-tracking branch 'upstream/master'

abd6c2d

introduction of symbolic binary class - no more decoder class

2c7284a

minor bugs fixed, applies binary rules to input now

cf2c15a

started programming the BinaryCode class, work in progress

1e81d79

added count qubit function in binary operator, modified code.py

6567ff0

fixed morning brain bugs

d7817bd

implemented concatination and appending, not yet documented and debugged

88cb489

added _shift function to symbolicBinary and integer addition. modifie…

103cd73

…d the decoder shifter accordinly

radd imul, NOT TESTED! - just copy paste should work?

88abd7a

fixed small bugs, added default codes, created transform

a1ab95b

fixed the multiplication error and modified the transform code follow…

684c6af

…ing mathematica code. it was giving wrong operators before, now the operators are fine but signs are wonky

fixed the transform and all the places where we accidentally started …

372e6e6

…to count qubit/fermion labels from 1 instead of 0

bug fixes: ordering matters to detect which terms should cancel

203035e

fixed code appending, introduced the integer multiplication (left+rig…

f2996ca

…ht) as a tool to append the same code intance several times

Merge remote-tracking branch 'upstream/master'

a09835c

Merge branch 'master' of https://github.com/conta877/OpenFermion

4be14a9

modified the multiply by 0 and 1 behavior in SymbolicBinary. added te…

76562f6

…sts for symbolicBinary. minor mods for python3 comp.

fixed the condition for code concatenation, fixed a bug in checksum_c…

5c962a9

…ode, added an error if qubit index in a code is out of bounds

added comments to binary_operator, more test, evaluate function

84521d5

started writing documentations and clipping lines, writing out numpy

c9c123c

merging with the merger

33ce89e

babbush reviewed Mar 3, 2018

View reviewed changes

kevinsung reviewed Mar 3, 2018

View reviewed changes

Merge branch 'master' into master

237d649

Merge branch 'master' into master

1c49f56

kevinsung mentioned this pull request Mar 3, 2018

started features list #221

Closed

conta877 and others added 5 commits March 3, 2018 16:14

Merge remote-tracking branch 'upstream/master'

986ce62

not ready for review! started on docs

34ffef3

Merge branch 'master' into master

aad3a4e

descriptions for code functions

1be05b9

final pass at docs, fixing spaces

46b1089

babbush suggested changes Mar 5, 2018

View reviewed changes

conta877 added 2 commits March 5, 2018 11:31

modes to n modes, and imports

c1fda57

fixed a bug in demo, added description to weight-n codes

8cf9dc9

babbush merged commit 8856404 into quantumlib:master Mar 5, 2018

kevinsung mentioned this pull request Mar 16, 2018

Which Bravyi-Kitaev transform? #259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

4/4! - transform functions. #232

4/4! - transform functions. #232

conta877 commented Mar 2, 2018

babbush Mar 3, 2018

babbush left a comment

babbush Mar 3, 2018

kevinsung Mar 3, 2018

idk3 Mar 3, 2018

babbush Mar 3, 2018

conta877 Mar 3, 2018

msteudtner Mar 3, 2018

kevinsung Mar 3, 2018

msteudtner Mar 3, 2018

kevinsung Mar 3, 2018

kevinsung Mar 3, 2018

kevinsung Mar 3, 2018

conta877 commented Mar 3, 2018 via email

kevinsung commented Mar 3, 2018

babbush commented Mar 3, 2018

babbush commented Mar 3, 2018

kevinsung commented Mar 3, 2018 •

edited

kanavsetia commented Mar 3, 2018

babbush commented Mar 3, 2018

kevinsung commented Mar 3, 2018

babbush commented Mar 3, 2018

kevinsung commented Mar 3, 2018

conta877 commented Mar 3, 2018

conta877 commented Mar 5, 2018

babbush left a comment

babbush Mar 5, 2018

babbush Mar 5, 2018

babbush Mar 5, 2018

babbush Mar 5, 2018

babbush Mar 5, 2018

conta877 commented Mar 5, 2018



		def _encoder_checksum(modes):
		""" Outputs the encoder matrix for checksum codes.


		def weight_one_binary_addressing_code(exponent):
		""" Weight-1 binary addressing code.

		from openfermion.ops import BinaryCode, SymbolicBinary, linearize_decoder


		def _encoder_bk(dimension):

		return BinaryCode(encoder, decoder)


		def weight_one_segment_code():

		['w0 w1 + w0', 'w0 w1 + w1', ' w0 w1'])


		def weight_two_segment_code():

4/4! - transform functions. #232

4/4! - transform functions. #232

Conversation

conta877 commented Mar 2, 2018

Choose a reason for hiding this comment

babbush left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conta877 commented Mar 3, 2018 via email

kevinsung commented Mar 3, 2018

babbush commented Mar 3, 2018

babbush commented Mar 3, 2018

kevinsung commented Mar 3, 2018 • edited

kanavsetia commented Mar 3, 2018

babbush commented Mar 3, 2018

kevinsung commented Mar 3, 2018

babbush commented Mar 3, 2018

kevinsung commented Mar 3, 2018

conta877 commented Mar 3, 2018

conta877 commented Mar 5, 2018

babbush left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conta877 commented Mar 5, 2018

kevinsung commented Mar 3, 2018 •

edited