search

ProjectQ-Framework / FermiLib

FermiLib: Open source software for analyzing fermionic quantum simulation algorithms
https://projectq.ch/
Apache License 2.0
87 stars 39 forks source link

Extract common helper from fourier_transform / inverse_fourier_transform #39

Closed Strilanc closed 7 years ago

Strilanc commented 7 years ago

They're large almost identical methods, so I merged them into a helper then called it with slightly different arguments.

I did it in two stages:

  1. Making the same except for assignments at the top. I used rename refactorings to fix differences in variable names. I fixed differences in logic by depending on the assignment values instead of hard-coded functions. I confirmed the function tails were exactly identical at this point by pasting one over the other and confirming no diff.

  2. Moving the common bodies into a method then calling it.

Strilanc commented 7 years ago

This is actually a subset of another PR that you approved...?

There were two big complicated methods, but they were almost identical. Any bugfix you make to one would have to be repeated on the other. Any feature you added to one would also be added to the other. There's no reason to do all that work, and all that reading, twice. This pulls out their common structure so that changes only have to be made once.

babbush commented 7 years ago

Ok, just change the names as I mentioned and I will approve.

Strilanc commented 7 years ago

Is the name 'position vector' / 'momentum vector' appropriate? When doing the inverse transform they would be the opposite of what they should be. I'm very happy to use good names instead of generic ones, and I defer to your expertise on knowing the better names for these methods, but we should avoid directly misleading names.

Strilanc commented 7 years ago

@babbush Please suggest alternative names (or just say to go with pos/mom) so that this can get pushed; it will have merge conflicts with some of the grid changes.