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

two methods of half-up #218

Merged
merged 70 commits into from
Feb 26, 2018
Merged

two methods of half-up #218

merged 70 commits into from
Feb 26, 2018

Conversation

conta877
Copy link
Contributor

@conta877 conta877 commented Feb 25, 2018

this pull request is more of a question

there are two implementations of half-up (#216),

  1. a stand alone function in fermion_operator.py; half_up_order

  2. modified get_molecular_hamiltonian function of MolecularData, which kinda spread from there -> InteractionRDM expectation needs to know the ordering of the InteractionOperator, that required the InteractionOperator to know its order. For now I only made expectation function raise NotImplementedError if InteractionOperator has half_up order, just because I am not sure which method we are gonna choose.

I am aware that 1) will cause the same problem with InteractionRDM but half_up_order function reordering feels less internal thus maybe its ok to not handle that.

Question is, which one do we want, do we want to implement this in another way?

Ryan edit: Closes #216

conta877 and others added 30 commits January 18, 2018 09:36
…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
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@kevinsung
Copy link
Collaborator

With method (2), do InteractionOperator and InteractionRDM really need to "know its order"? I think we can just assume that the user will only evaluate expectation values for operators that use the same ordering; otherwise it's her own fault. Therefore I think method (2) should only require modifications to MolecularData.

@conta877
Copy link
Contributor Author

@kevinsung no they don't need to know 2) can have the same attitude towards user errors. With that assumption, would you rather 1 or 2?

@babbush
Copy link
Contributor

babbush commented Feb 25, 2018

Agree with @kevinsung .

@kevinsung
Copy link
Collaborator

I think 2 is more user-friendly so I say we go with that.

@kevinsung
Copy link
Collaborator

I'll add that I don't think half_up is a descriptive enough name. I suggest that we actually call the argument spin_indexing and set it to 0 or 1, and clearly document that, for instance, 0 indicates the even-odd scheme and that 1 indicates the up-then-down scheme.

@kevinsung
Copy link
Collaborator

kevinsung commented Feb 25, 2018

Actually, instead of spin_indexing be 0 or 1, perhaps let's make it be a string, either 'even-odd' or 'up-then-down'.

@conta877
Copy link
Contributor Author

what do you think of indexing = "orbital" or "spin"

@babbush
Copy link
Contributor

babbush commented Feb 25, 2018

If we're going to go with a string I like Kevin's suggestion because it leaves no ambiguity as to what it does.

@conta877
Copy link
Contributor Author

ignore my last push - I failed at properly merging with my master.

@babbush
Copy link
Contributor

babbush commented Feb 26, 2018

Let me know when you're ready for review.

@conta877
Copy link
Contributor Author

conta877 commented Feb 26, 2018 via email

Copy link
Contributor

@babbush babbush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent contribution. Thanks for taking care of this!

@babbush babbush merged commit 761b168 into quantumlib:master Feb 26, 2018
philipp-q pushed a commit to philipp-q/OpenFermion that referenced this pull request Sep 2, 2020
* 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
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

Successfully merging this pull request may close these issues.

None yet

5 participants