search

babbush / HistoricalFermiLib

This is repo where we developed FermiLib, which then became OpenFermion
Apache License 2.0
1 stars 0 forks source link

Python 3 compatibility issue with LocalOperator's and LocalTerm's / and /= #35

Closed damiansteiger closed 7 years ago

damiansteiger commented 7 years ago

There are compatibility issues with __div__ and __idiv__ method in LocalOperator and LocalTerm for Python 3.3+.

The reason is that in Python2.7 / is floor division for integers while in Python3.3+ / is the approximate floating point division also for integers.

These problems could be solved by using from __future__ import division and implementing in addition the __truediv__ and __itruediv__ method.

However, sticking with our current implementation of __div__ and __idiv__, the syntax will be "unpythonic" as QubitTerm('X1 Y3 Z8') / 2 is currently implemented as floating point division but in Python2 / should be floor division. However, in Python3 QubitTerm('X1 Y3 Z8') / 2 should be floating point division. I think that only creates confusion and therefore, I would suggest to remove support for __div__ and __idiv__ as users can use __mul__ which is unambiguous.

What do people think?

idk3 commented 7 years ago

You could fix this by forcing coefficient to be floating point, but I don't think it's a very big issue.. "x / 6" is much nicer than having to do "x * (1. / 6)".

idk3 commented 7 years ago

(I think forcing coefficient to be floating point is the way to fix it, I mean.)

damiansteiger commented 7 years ago

Yes, treating integers as floating points in both python 2 and python 3 would work. I am just wondering if we would confuse Python people by doing that...

Regarding your example in which you want floating point division: x / 6 By removing __div__ and using __mul__ only: In Python2 the above statement is equivalent to: 1./6 * x If you use from __future__ import division in Python2 the above statement is equivalent to: 1/6 * x And in Python3 it is equivalent to: 1/6 * x All of these cases are just as expected in the corresponding Python version.