Improve mechanism for choosing spin indexing scheme about openfermion HOT 32 CLOSED

I say we leave it to the user and just document somewhere our chosen spin-indexing convention. @ncrubin are you okay with removing the spin-indexing keyword arguments from the routines for generating spin operators and Hamiltonians?

from openfermion.

I think I'm still on the side of keeping a consistent internal convention for ordering. I worry about adding too much to FermionOperator which would reduce it's abstractness. As for the spin-less fermion example cited by @kevinsung, I think it's a great example of something where the current objects can be easily used (adding an "ordering" would require the user to stub out that feature). I worry that an "ordering aware" InteractionOperator and FermionOperator greatly increases the API complexity. Each option is a promise we are making to maintain for all features pertaining to objects. I just don't think that's necessary.

from openfermion.

EDIT: After flip flopping a bit I decided to stick with my proposal above.

from openfermion.

For hamiltonians defined in the context of action on spin-full orbitals (e.g. spatial sites for Hubbard ) then using the string ordering in #218 is fine but I don't see how you would maintain a separation between the MolecularData object and hamiltonians (FermionOps). The "even-odd" or "up-then-down" etc... seems to be tightly coupled with the Molecular data object and not FermionOperator. I know the previous discussion opted for this route to reduce the code overhead in FermionOperator (which I think is fine) but I believe necessitates a function map for spin-ordering to maintain a good abstraction separation. One solution could be to add a function that returns the up-map or down-map in 'even-odd' 'up-then-down'etc ordering.

As for the hamiltonians/._special_operators and utils/_sparse_tools I feel the function maps should be there. Since these functions rely explicitly on knowledge of where the up-spin/down-spin are located in the number occupation vector (or the appropriate rep in parity vector or parity-tree vector--I'm thinking of the strings post BK transform). I would opt for someone explicitly specifying how to take an index and map that to the up or down fermion.

from openfermion.

@ncrubin can you imagine any other ordering one might want to explore other than even_odd or up_then_down?

from openfermion.

from openfermion.

@babbush I was imagining something like @jarrodmcc described but had not worked out an explicit example. In this fictitious example, I pictured an ordering scheme that helped map to various physical qubit topologies given a particular transform. But I agree with @jarrodmcc that the place for the mapping is at the interface to qubit Ops not internal to the workings of OpenFermion. I really like the idea of standardizing an ordering internal to OpenFermion; It'll make writing transforms easier and lower the barrier of entry code contributions.

from openfermion.

So the suggesting here is to always enforce a certain convention in the order of FermionOperators (for instance, always interleave spin up and spin down)? And then all transforms should come with the option to change the ordering with a mapping function? If that is the suggestion, I am okay with it if somebody wants to implement.

from openfermion.

I think this suggestion is fine, but note that it would involve undoing the changes made in #218. That PR actually leaves some things hanging because it introduces the possibility of InteractionOperators which use differing spin conventions, and the current implementation of rotate_basis only supports one convention.

My suggestion at the top of this thread is actually in the same spirit, but it is that we support two internal conventions, rather than just one. Considering the issue I just mentioned regarding rotating bases for InteractionOperators, I believe the best way to implement this would be to have data structures including InteractionOperator and FermionOperator "be aware" of their spin indexing, i.e., add an attribute to indicate whether or not spin is being represented, and if so which convention is being used. @babbush I know you and I both expressed resistance to this before but I've relaxed my stance. I think this could be useful and that the conventions would be easy to maintain together. For instance, the basis rotation code would only need a very simple update.

I'm still undecided on which way is better. Adding more attributes is kind of icky but I do think it could be useful, and I think it'll be necessary if we keep the changes from #218.

from openfermion.

I still disagree that it makes sense to keep track of spin in FermionOperator. I think we should go with one convention for all routines and then give people the option to change the ordering at the time of mapping to qubits. The reason why is because it never matters what the order is until one maps to qubits. So I think it just complicates code unnecessarily to even give people the option of the changing the ordering prior to the mapping to qubits. @ncrubin @jarrodmcc what do you think?

from openfermion.

Sure, the order does not matter in FermionOperator, but it could matter in InteractionOperator or other subclasses of PolynomialTensor. For instance, maybe in one ordering the one-body tensor is block diagonal. However, we can take care of this by providing a basis change matrix to switch between the orderings, so I guess it's not necessary to keep track of an attribute.

from openfermion.

But why do you care if the one-body tensor is block diagonal? Why does that matter? I am tryinig to understand why it might be useful to reorder PolynomialTensor. I'm not sure I see it.

from openfermion.

For instance, to take advantage of spin symmetry in parallelizing the Givens rotation algorithm for preparing Slater determinants, we need to use the up-then-down ordering so the one-body tensor is block diagonal. Another advantage is that orbitals of different spin don't get mixed together, so this would be helpful for preparing Slater determinants with fixed spin.

from openfermion.

I disagree with your example @kevinsung . You could have written the Givens rotation algorithm to just focus on diagonalizing the matrix by only ever rotating between orbitals of the same spin. It does not matter their order. Then, when mapping to qubits, if you choose the up then down ordering you will find that your Givens rotations are perfectly parallelized between up and down spin.

from openfermion.

Well, then it would need to be aware whether spin is being represented at all, and if so, whether it is possible to diagonalize the matrix by only rotating orbitals of the same spin. For instance, this is possible for the hopping term of the Hubbard model but I don't think it's possible for the BCS Hamiltonian.

from openfermion.

So then there should be a flag in the Givens rotation code that says "spin_symmetry = True/False".

from openfermion.

That's a fine suggestion! You all have convinced me. Perhaps we should loop in @conta877 here because it looks like we've agreed to actually choose option (1) of what he originally proposed in #218. Ryan summarized it above as

So the suggesting here is to always enforce a certain convention in the order of FermionOperators (for instance, always interleave spin up and spin down)? And then all transforms should come with the option to change the ordering with a mapping function? If that is the suggestion, I am okay with it if somebody wants to implement.

I think this might be slightly different, but does it satisfy your needs @conta877 ?

from openfermion.

@conta877 is a "she". And yes, it is closer to that suggestion. Sorry @conta877 ! To reiterate, I think the way to do this is to have a single "mapping" function which takes FermionOperator as input and reorders the modes. Perhaps this should be a "private" function that is the first step in all fermion to qubit transforms. Then, the fermion to qubit transforms should all have an optional parameter to reorder the modes. There is a question of whether this should be a function that is passed or a string argument. The advantage of a function is that it allows for arbitrary re-orderings. The advantage of a string is that it is simple.

I'll mark this as an open issue. Thanks for the discussion everyone.

from openfermion.

I can start by reverting to (1) and adding a toggle to all transform functions. I like the idea of arbitrary re-ordering but is it too much to ask from a user?

from openfermion.

Perhaps we should implement the arbitrary reorder function and use it to define a function called "up_then_down_map". So the user can always provide that if it's what they want. But if they want something more sophisticated, like what @jarrodmcc suggested, they can also do that.

from openfermion.

Ok - sounds good! Where should these mapping functions live?

from openfermion.

Probably in utils or transforms somewhere. Suggestions welcome!

from openfermion.

@babbush @conta877 @jarrodmcc I do have some code from earlier that could be built upon without too much effort to do arbitrary reordering (mentioned in #216). The basic principle is fairly simple - you use fermionic swaps to implement a sorting algorithm. You can sort to go from input_order to [0, 1, 2, ..., n], and then de-sort (reverse the operations of the sort) to go from [0, 1, 2, ..., n] to an arbitrary final_order. In the past I did this with parallel bubble sort.

from openfermion.

@idk3 that is not needed here. This is before mapping to qubits so it's nearly trivial to do these reorderings. One needs only to relabel the modes.

from openfermion.

@babbush fair - my case has the mapping order already imposed. In that case the only advantage to my procedure is avoiding having to redo the normal ordering (which wouldn't be very expensive anyway if the operator is already normal-ordered).

from openfermion.

There is no reason to normal order if we are about to map to qubits. The mapping to qubits is unique.

from openfermion.

@idk3 I was normal ordering just to make sure my transforms work (visual inspection) - forgot to remove it afterwards.

from openfermion.

@kevinsung @conta877 do we feel that #224 closed this issue?

from openfermion.

we were talking about transforms handling the ordering - I don't know if I like the idea of introducing an order_function variable to every single transform, but I'll implement it if thats what everyone prefers.

from openfermion.

The one last thing is that the Hamiltonian generators should no longer have the optional arguments up_map and down_map. Do you agree @ncrubin ?

from openfermion.

whats the situation on this one; do we want to add an option to transforms or should we leave it to the user to reorder before sending it in?

from openfermion.

I suggest doing this and just adding a note about our conventions somewhere in get_molecular_hamiltonian() or something like that. This would close this issue.

from openfermion.