spin indexing #219 #224

conta877 · 2018-02-27T20:33:03Z

what do we think of something like this? #219

no docstrings or descriptions yet - I'll add them once everyones happy :)

created the order functions such that they take in the mode_idx and number of modes as arguments.
there is a lonely order_function (for up-then-down) under transforms/order_functions

here is a sample output:

from openfermion.transforms import get_fermion_operator
from openfermion.transforms import up_then_down
from openfermion.ops import normal_ordered


geometry = [("H", (0., 0., 0.)), ("H", (0., 0., 0.6))]
molecule = MolecularData(geometry, "sto-3g", 1,
                         description="0.6")
molecule.load()
molecular_hamiltonian = molecule.get_molecular_hamiltonian()
hamiltonian = normal_ordered(get_fermion_operator(molecular_hamiltonian))

print hamiltonian
print "\n----------------------\n"
print hamiltonian.reorder(order_function=up_then_down)
print "\n----------------------\n"
print hamiltonian.reorder(num_modes = 8, order_function=up_then_down)


0.17373064388 [3^ 0^ 2 1] +
-0.365770571815 [2^ 2] +
0.17373064388 [2^ 1^ 3 0] +
-0.515062453042 [3^ 1^ 3 1] +
-0.17373064388 [1^ 0^ 3 2] +
-0.365770571815 [3^ 3] +
-0.701337728655 [1^ 0^ 1 0] +
-0.688793096922 [2^ 1^ 2 1] +
-0.17373064388 [3^ 2^ 1 0] +
0.881962014317 [] +
-0.515062453042 [2^ 0^ 2 0] +
-0.724506023965 [3^ 2^ 3 2] +
-1.34221399305 [0^ 0] +
-1.34221399305 [1^ 1] +
-0.688793096922 [3^ 0^ 3 0]

----------------------

-0.515062453042 [1^ 0^ 1 0] +
-1.34221399305 [2^ 2] +
-0.17373064388 [2^ 0^ 3 1] +
0.17373064388 [3^ 0^ 1 2] +
-0.724506023965 [3^ 1^ 3 1] +
-0.365770571815 [3^ 3] +
-0.17373064388 [3^ 1^ 2 0] +
-0.688793096922 [1^ 2^ 1 2] +
0.881962014317 [] +
-0.701337728655 [2^ 0^ 2 0] +
-0.515062453042 [3^ 2^ 3 2] +
-1.34221399305 [0^ 0] +
-0.365770571815 [1^ 1] +
-0.688793096922 [3^ 0^ 3 0] +
0.17373064388 [1^ 2^ 3 0]

----------------------

0.17373064388 [5^ 0^ 1 4] +
-1.34221399305 [4^ 4] +
-0.688793096922 [1^ 4^ 1 4] +
-0.365770571815 [1^ 1] +
-0.17373064388 [5^ 1^ 4 0] +
-0.515062453042 [5^ 4^ 5 4] +
-0.17373064388 [4^ 0^ 5 1] +
-0.724506023965 [5^ 1^ 5 1] +
-0.701337728655 [4^ 0^ 4 0] +
0.17373064388 [1^ 4^ 5 0] +
-0.515062453042 [1^ 0^ 1 0] +
0.881962014317 [] +
-0.365770571815 [5^ 5] +
-1.34221399305 [0^ 0] +
-0.688793096922 [5^ 0^ 5 0]

this was quick prototyping haven't really tested it yet, please let me know if you spot bugs!

…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-02-28T01:04:45Z

@conta877 let me know when you are ready for review.

conta877 · 2018-02-28T01:40:07Z

I'm ready for input. (not review). Is this the way we want to proceed?

…

On Feb 27, 2018 17:04, "Ryan Babbush" ***@***.***> wrote: @conta877 <https://github.com/conta877> let me know when you are ready for review. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#224 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AV-mvIdinJMhSH-MbcgMEJ4SE04333jhks5tZKYvgaJpZM4SVmSG> .

babbush · 2018-02-28T01:41:36Z

I think so. It would be good hear from at least two of either:
@ncrubin @kevinsung @jarrodmcc to make sure they agree since all three seemed to have opinions...

conta877 · 2018-02-28T01:47:53Z

I also think integration of this into transforms should be a separate pull request so that can discuss.

…

On Feb 27, 2018 17:41, "Ryan Babbush" ***@***.***> wrote: I think so. It would be good hear from at least two of either: @ncrubin <https://github.com/ncrubin> @kevinsung <https://github.com/kevinsung> @jarrodmcc <https://github.com/jarrodmcc> to make sure they agree since all three seemed to have opinions... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#224 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AV-mvK0oaSbwWPjVKtLWUTJrKcpUWUgSks5tZK7SgaJpZM4SVmSG> .

kevinsung · 2018-02-28T03:22:25Z

This looks fine to me.

jarrodmcc · 2018-02-28T05:19:19Z

I think this type of re-order will work well for most of the use cases I'm thinking of. I know you said you'd clean up the docstring and unused parameter stuff after we agreed on the style, but I wanted to make sure I mentioned something slightly more subtle, as it looks like when an order function is passed a copy of the modified operator is returned, whereas when no order function is passed, a reference to the original is returned. I feel like this could lead to slightly unexpected behavior in some cases. In my opinion it's okay to make the reorder function parameter not optional. If they don't want to reorder it, why risk unexpected behavior by letting them call it.

…

On Tue, Feb 27, 2018 at 7:22 PM, Kevin J. Sung ***@***.***> wrote: This looks fine to me. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#224 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHXFPLrtYejXViHpiLyGJ5FWmT5MRcGFks5tZMZzgaJpZM4SVmSG> .

conta877 · 2018-02-28T06:23:03Z

thats a really good point! I'll modify it to make the order function a must. On Feb 27, 2018 21:19, "Jarrod" <notifications@github.com> wrote: I think this type of re-order will work well for most of the use cases I'm thinking of. I know you said you'd clean up the docstring and unused parameter stuff after we agreed on the style, but I wanted to make sure I mentioned something slightly more subtle, as it looks like when an order function is passed a copy of the modified operator is returned, whereas when no order function is passed, a reference to the original is returned. I feel like this could lead to slightly unexpected behavior in some cases. In my opinion it's okay to make the reorder function parameter not optional. If they don't want to reorder it, why risk unexpected behavior by letting them call it.

On Tue, Feb 27, 2018 at 7:22 PM, Kevin J. Sung ***@***.***> wrote: This looks fine to me. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#224 (comment) , or mute the thread <https://github.com/notifications/unsubscribe-auth/

AHXFPLrtYejXViHpiLyGJ5FWmT5MRcGFks5tZMZzgaJpZM4SVmSG>

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#224 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AV-mvDWH2iyLTtu97fmx1_nSPJeSGOBzks5tZOHZgaJpZM4SVmSG> .

…into mergehalfup

conta877 · 2018-02-28T19:55:03Z

is it worth having a separate place for the order functions or should they be put in somewhere else? up_then_down looks very lonely there right now.

kevinsung · 2018-02-28T23:27:35Z

I suggest that for now, we keep both the functions reorder and up_then_down in the same file, either utils/_operator_utils.py or transforms/_conversion.py. I don't feel very strongly though.

jarrodmcc · 2018-03-01T01:19:45Z

I would agree that until we have more functions of this type (if we make them) they should just go alongside the functions they are meant to be used with for clarity instead of their own file.

…

On Wed, Feb 28, 2018 at 3:27 PM, Kevin J. Sung ***@***.***> wrote: I suggest that for now, we keep both the functions reorder and up_then_down in the same file, either utils/_operator_utils.py or transforms/conversion.py. I don't feel very strongly though. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#224 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHXFPIyorES8CmKNT4OIOmrEL_UmDE0wks5tZeDpgaJpZM4SVmSG> .

conta877 · 2018-03-01T01:36:01Z

ready for review

kevinsung · 2018-03-01T04:30:26Z

Thank you @conta877! In addition to the inline comments I have two general things:

If you haven't done so, please run the PEP8 linter on your code and fix the style violations. You can install it with pip install pycodestyle.
So the reverse flag should have the property that if I reorder a FermionOperator twice in a row with the same parameters, except once with reverse=False and once with reverse=True, then I should get the same thing back? Can you add a test for this for the up_then_down_map?

kevinsung · 2018-03-01T04:25:17Z

src/openfermion/utils/_operator_utils.py

@@ -290,3 +290,61 @@ def save_operator(operator, file_name=None, data_directory=None,
    with open(file_path, 'wb') as f:
        marshal.dump((operator_type, dict(zip(tm.keys(),
                                          map(complex, tm.values())))), f)
+
+
+def reorder(Operator, order_function,num_modes=None, reverse=False):


Here I would use lowercase to conform with our general style (operator instead of Operator). We generally use lowercase for variables.

kevinsung · 2018-03-01T04:26:31Z

src/openfermion/utils/__init__.py

@@ -21,7 +21,8 @@

 from ._operator_utils import (count_qubits, eigenspectrum, fourier_transform,
                              get_file_path, inverse_fourier_transform,
-                              is_identity, load_operator, save_operator)
+                              is_identity, load_operator, save_operator,
+                              reorder,up_then_down)


Missed a space. Please run the PEP 8 linter.

conta877 · 2018-03-01T17:51:33Z

@kevinsung thanks for the review! - I caught a bug putting in the reverse test.

kevinsung

Just one last thing I noticed.

kevinsung · 2018-03-01T18:21:00Z

src/openfermion/utils/_operator_utils.py

@@ -15,8 +15,6 @@

 import marshal
 import numpy
-import os
-import time



Thanks for removing time, but I think os is in fact used in the saving operation; apparently we don't have a test for that line right now...

oops! I got tricked by pycharm. will add it back

kevinsung · 2018-03-01T18:36:43Z

Thanks @conta877 ! It turns out we did have that test (your pycharm is not crazy) but it passed due to namespace pollution from the * imports, which will be fixed in another PR. Anyway I think this is ready to be merged.

babbush · 2018-03-01T18:46:20Z

src/openfermion/hamiltonians/_molecular_data.py

-                                  active_indices=None,
-                                  spin_indexing="even-odd"):
+    def get_molecular_hamiltonian(self,occupied_indices=None,
+                                      active_indices=None):


this spacing seems to be inconsistent with pep8. No?

indeed. I'm failing at pep8ing with this commit.

I figured we had given you enough trouble. Perhaps clean it up on the next PR if you're feeling charitable ;-)

* 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 * proper master reset * reverting spin_indexing to option 1 with arbitrary functions * moving reorder under operator_utils and making it symbolicOperator general * +1 bug fix * docstrings, variable changes, tests * coverage * put up_then_down with reorder * bug fixes, pep8 * adding os back

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

Merge branch 'master' into mergehalfup

79a53e0

conta877 added 3 commits February 28, 2018 11:28

docstrings, variable changes, tests

ff04e0e

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

654f059

…into mergehalfup

coverage

a8fdc62

put up_then_down with reorder

e408bdb

kevinsung requested changes Mar 1, 2018

View reviewed changes

babbush and others added 2 commits February 28, 2018 22:56

Merge branch 'master' into mergehalfup

f6614f1

bug fixes, pep8

73e1331

kevinsung approved these changes Mar 1, 2018

View reviewed changes

kevinsung requested changes Mar 1, 2018

View reviewed changes

adding os back

3827e6b

kevinsung approved these changes Mar 1, 2018

View reviewed changes

babbush approved these changes Mar 1, 2018

View reviewed changes

babbush merged commit a0a43a5 into quantumlib:master Mar 1, 2018

babbush mentioned this pull request Mar 1, 2018

Improve mechanism for choosing spin indexing scheme #219

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

spin indexing #219 #224

spin indexing #219 #224

conta877 commented Feb 27, 2018 •

edited

Loading

babbush commented Feb 28, 2018

conta877 commented Feb 28, 2018 via email

babbush commented Feb 28, 2018

conta877 commented Feb 28, 2018 via email

kevinsung commented Feb 28, 2018

jarrodmcc commented Feb 28, 2018 via email

conta877 commented Feb 28, 2018 via email

conta877 commented Feb 28, 2018 •

edited

Loading

kevinsung commented Feb 28, 2018 •

edited

Loading

jarrodmcc commented Mar 1, 2018 via email

conta877 commented Mar 1, 2018

kevinsung commented Mar 1, 2018

kevinsung Mar 1, 2018

kevinsung Mar 1, 2018

conta877 commented Mar 1, 2018

kevinsung left a comment

kevinsung Mar 1, 2018

conta877 Mar 1, 2018

kevinsung commented Mar 1, 2018

babbush Mar 1, 2018

conta877 Mar 1, 2018

babbush Mar 1, 2018

spin indexing #219 #224

spin indexing #219 #224

Conversation

conta877 commented Feb 27, 2018 • edited Loading

babbush commented Feb 28, 2018

conta877 commented Feb 28, 2018 via email

babbush commented Feb 28, 2018

conta877 commented Feb 28, 2018 via email

kevinsung commented Feb 28, 2018

jarrodmcc commented Feb 28, 2018 via email

conta877 commented Feb 28, 2018 via email

conta877 commented Feb 28, 2018 • edited Loading

kevinsung commented Feb 28, 2018 • edited Loading

jarrodmcc commented Mar 1, 2018 via email

conta877 commented Mar 1, 2018

kevinsung commented Mar 1, 2018

kevinsung Mar 1, 2018

Choose a reason for hiding this comment

kevinsung Mar 1, 2018

Choose a reason for hiding this comment

conta877 commented Mar 1, 2018

kevinsung left a comment

Choose a reason for hiding this comment

kevinsung Mar 1, 2018

Choose a reason for hiding this comment

conta877 Mar 1, 2018

Choose a reason for hiding this comment

kevinsung commented Mar 1, 2018

babbush Mar 1, 2018

Choose a reason for hiding this comment

conta877 Mar 1, 2018

Choose a reason for hiding this comment

babbush Mar 1, 2018

Choose a reason for hiding this comment

conta877 commented Feb 27, 2018 •

edited

Loading

conta877 commented Feb 28, 2018 •

edited

Loading

kevinsung commented Feb 28, 2018 •

edited

Loading