Bksf

[kanavsetia]

I have added the files for the BKSF algorithm. _bksf.py contains all the methods for implementing BKSF algorithm and _test_bksf.py contains the test routines. Suggestion and feedback about more tests are welcome.

[idk3]

Hey @kanavsetia, I noticed your test file isn't being picked up by coveralls - could you change its name from _test_bksf.py to _bksf_test.py? That should fix it.

[jarrodmcc]

I'm still reading through the other parts of the code but make sure to add imports for all routines you want exposed to users into init.py within transforms as well, so they can be easily imported externally

[babbush]

Kanav, take your time with this but let us know too if you need us to help suggest some fixes if you are unable to get the tests to pass.

[kanavsetia]

Actually Ryan, I can use some help. The tests are working fine on my computer. I checked the error from travis CI, and it say it's not able to import _bksf. I feel like I am making some really silly error and it has to do with the way python packages and modules work. Could you look at my " import _bksf" statement and suggest a change? Thanks!

[thomashaener]

Try from . import _bksf instead. Also, when running pytest locally on your machine, make sure you are in the main fermilib directory (and not in, e.g., transforms).

[babbush]

Actually, what I would suggest is that you edit the file transforms/__init__.py. In that file, add: from ._bksf import (FUNCTIONS) where FUNCTIONS is the list of functions you will need.

Then, whenever you want to use those functions you can just type from fermilib.transforms import FUNCTIONS

Note that you should not import internals of the library here (e.g. helper functions), just functions that users might want to use. Thus, in your test file you may need to import the helper functions separately by writing what Thomas suggested or instead writing: from ._bksf import (HELPER FUNCTIONS)

Let me know if that makes sense.

[kanavsetia]

Thank you @thomashaener for your input, but for some reason it is not working. Even when I am in the fermilib directory, import _bksf seems to be working fine but from . import _bksf is giving me an error (Parent module not loaded, cannot perform relative import). I am having some problems with pytest on my machine too. Apparently it is not able to import even scipy.

[kanavsetia]

Thanks Ryan, I will update the init file in the transforms directory. But that doesn't seem to be the problem when running the tests. Since, it is in the same directory as _bksf.py, I would've assumed import _bksf to have worked. And, as I told Thomas, for some reason from . import _bksf is not working for me.

[thomashaener]

It should work if you put it below all other fermilib imports (fermilib has to be loaded before the relative import).

[kanavsetia]

Nope, that doesn't work either. Is there a file where you have to declare all the modules so the interpreter knows what modules are available in the package. I remember similar thing in manifest file for Android programming. But, I have checked Manifest, src, init, and some other files, I didn't find a file which had names of all the modules.

[babbush]

Hi Kanav,

You can check to see if it is going to work by reinstalling FermiLib with your PR in it and then running the tests. This might help you with debugging.

[thomashaener]

That's strange. I just cloned your repo and changed the import to a relative import below all fermilib imports and it worked... have you tried pushing the change to see if it works on travis?

import [file] works for files in your current directory or PYTHONPATH.

[thomashaener]

Great, now it's working. All that is left to do is add networkx to the requirements.txt file.

[kanavsetia]

Thanks, that was going to be my next questions. I just added the requirement.

[kanavsetia]

The log shows that tests passed for all the python versions except python 2.7. I looked at the assert statement that failed and I am not sure what would make it fail just for python 2.7. Any suggestions?

[damiansteiger]

Change

self.assertTrue(evensector == 2**(n_qubits - 1))

to

self.assertEqual(evensector, 2**(n_qubits - 1))

I think it is like this at two places. This will give a better error message, in your case:

self.assertEqual(evensector, 2**(n_qubits - 1))
E       AssertionError: 5 != 8

I hope this helps finding the error...

[kanavsetia]

Thanks Damian! Also, is there a way to change the code and run travis CI without committing?

[damiansteiger]

Travis is our online continuous integration testing. It only runs for commits on FermiLib (pull requests and branches).

You can run the tests locally. You need to have pytest installed (python2.7 -m pip install pytest), then navigate to the FermiLib folder and execute:

python2.7 -m pytest .

This will run the tests locally. Change python2.7 for your version of python2 and maybe run the tests similarly with a python3 version. This allows to locally debug and not having to wait for Travis and commit always to see if it works.

To speedup the debugging you might also just want to run your failing test individually:

python2.7 -m pytest src/fermilib/transforms/_bksf_test.py

[damiansteiger]

Ideally you would use a debugger to see what goes wrong, but you can also just insert print(variable) and if the test fails, pytest will output also the print statement (otherwise it is ignored). Just don't forget to remove all print statements as otherwise the reviewers are unhappy ;-)

[kanavsetia]

I do use debugger and print statements. And except for the first commit, I've been careful enough to delete them before submitting my code :) But pytest is new for me and for some reason it is not working properly, I am still trying to figure out what is wrong with it. Maybe little more time on stackexchange and I'll get it to work. Also, this specific error is interesting. I am not getting any error while running tests on my machine. On travis CI too, it gives an error just for python 2.7. If you any suggestions let me know. Also, thank you to all the reviewers for being patient with me.

[babbush]

No problem Kanav, thanks for being patient with us and going through this process. Indeed, it is quite rare to see tests fail in this manner for just one version. Usually when that happens it is pretty clear what is going on (like you're trying to use a function that doesn't exist in that version of python or something). But in this case, the code is running fundamentally differently in the different versions because you get a test that fails with "5 != 8". I think the best way to debug this is to use python 2.7 to track what's going on in the code and to figure out what's going wrong to give you a 5 instead of an 8. Basically just run the code with print statements everywhere until you figure out what line is doing something unexpected.

[babbush]

If I had to guess, I suspect something is going on with "true division" versus "classical division". In python 2.7 if you write 5/2 you get 2 because they are both ints so the division operator always returns an int. Whereas if you write 5 / 2.0 or 5 / 2. or 5. / 2. or 5.0 / 2 or 5. / 2 or 5.0 / 2.0 you get 2.5 because it realizes one of the numbers is a float. But in python 3, division is always true division so even 5 / 2 = 2.5. This is the sort of thing that might cause this type of discrepancy between versions. But I am really not sure.

[thomashaener]

That's right! The problem is in _bksf.py, line number 248: 1/8 should really be 1/8..

[kanavsetia]

Okay, I've spent yesterday evening, and today morning trying to get python 2.7 to work on my computer but it hasn't worked out well for me. Now, at least I have python 2.7 working, but for some reason my python 2.7 environment is not able to find projectq. So, for now, can I just make the change that Thomas suggested and add little more info about the test in the code (as suggested by Damian)? Meanwhile, I will try to get the python 2.7 up and running and write few more tests. Right now, the test coverage for _bksf.py is just 68% and I think I can write few more test cases to increase that coverage considerably. Also, thank you, Ryan, Thomas, and Damian, your inputs were really helpful!

[damiansteiger]

Sounds like a great plan 👍

• By changing to floating point as suggested by the others, all tests work fine on my machine. If you want I can change it for you, just let me know...
• Some documentation (maybe just a sentence or two for some of of the tests would be great, e.g., why and which part of the spectrum have to be identical to jordan wigner...)
• If you find time to write more tests that would be great but we can also help or point to places which need to be tested

Don't worry, there is no hurry. Let us know when you are done with some parts, need help, or some of us can also just spend some time to help directly with the code

If you have trouble with some installation, just post what OS your using and the current problem. We can give hints or suggestions to get it working on your machine.

[kanavsetia]

Thank you Damian!

• There were couple of more places where conversion was required. Those places didn't show up in the present tests but might have showed up later. So, hopefully I won't get a similar error later.
• I have added some documentation for the tests. Let me know if you think that more explanation is needed. Also, let me know, if more explanation is needed in the main code.
• I just wrote the test for generate_fermions(), so hopefully the test coverage will be little more. I am almost done with writing test for vacuum_state(), and I am going to try simulating new molecules over the next couple of days. I will use one of those and write new tests to push the test coverage to close to 100%.
• Also, thank you for offering help with the code and installation. I recently switched from windows to linux, so some of the installation procedures give me pain sometime. I may come back and ask for more help :)
• For the code, I am going to try and run the fermilib plugin for psi4 tomorrow, so I may ask you guys about it.

[damiansteiger]

Thanks for the changes.

I will have a short go at the code to fix a few things but first I will leave some comments so that you know why I am going to change these things. These are just cosmetic changes to adapt your code to the ProjectQ-Framework coding style...

Regarding tests: It would be great if most of the code is tested by unit tests (if not possible functional tests are also great). Unit tests test individual parts of the code separately. If you click below on the failing test coverage (coverage/coveralls) details you can find your new file and it shows in red which parts you have not tested. Try to think if import test cases which would cover these parts of the code. At the moment you can find this at: https://coveralls.io/builds/12550605/source?filename=src%2Ffermilib%2Ftransforms%2F_bksf.py

Functional tests on the other hand try to test all the individual pieces as a whole. They are great to make sure everything works fine but if they fail it is hard to figure out what is wrong. An example of a function test is your proposed test of a molecule. Such a test would be great! By convention, this test would go into src/fermilib/tests/.

Yes, definitely post questions about installation problems (if it is general, you can open an issue and someone can suggest a good way of installing things. Linux installations should be much easier than Windows)

[damiansteiger]

Code looks great Kanav👍 A few smaller things are left to change.

I will wait until there is enough test coverage by new tests and then a final pass and we have this new code in FermiLib :)

[babbush]

All checks have passed! I'll do a final review this afternoon!

[babbush]

I need to rerun tests as a result of updating the branch but this code does not interact with the last few PRs so it shouldn't be a problem. Once tests are complete I will merge. Kanav, please go ahead and send a photo of yourself to fermilib@projectq.ch so we can add to the website. As James is your adviser, we can also add James as a scientific adviser if he likes, he can send a photo as well.

[kanavsetia]

Hey Ryan, Thank you for merging my code. I will talk to James and send you the photo soon.