search

mcocdawc / chemcoord

A python module for manipulating cartesian and internal coordinates.
GNU Lesser General Public License v3.0
72 stars 19 forks source link

_jit_calc_singe_position as a static method #36

Closed sgill2 closed 7 years ago

sgill2 commented 7 years ago

In chemcoord.internal_coordinates._zmat_class_core.py you've recently implemented _jit_calc_positions as a static method for the ZmatCore class. It might make sense then to have _jit_calc_singe_position be a static method for that class as well, since they are related functions.

And on a minor related note, I think you probably intended to name the function _jit_cal_single_position, not _jit_calc_singe_position. That is, unless you wanted to slightly burn the molecule's position ;)

mcocdawc commented 7 years ago

Thanks for spotting the typo!

You would be completely right, if this was pure python code. The problem lies in @jit(nopython=True) as it is explained in the numba docs. Real "jitted" functions allow only ints, floats, strings, bools and arrays of the former. (A good rule of thumb is: If you can directly translate it into FORTRAN code, it can be nopython-mode jitted.) This especially means, that you cannot decorate normal methods with @jit(nopython=True), since the implicit first argument self is always a non-basic-type python instance. So you need normal functions or staticmethods.

It is important to know that _jit_calc_positions calls _jit_calc_single_position. Usually (without the restrictions of numba) _jit_calc_positions would be a normal method and _jit_calc_single_position a staticmethod. _jit_calc_positions could then just call self._jit_calc_single_position in its body. If they are both staticmethods _jit_calc_positions can't call normal or classmethods of itself. For this reason I have put _jit_calc_single_position into a namespace global to the class.

Now to conclude:

  1. I don't like my solution as well. If you come up with a better one, I happily accept PRs
  2. It seems as if I need a dedicated dev-doc to explain some design decisions, that deviate from normal idiomatic python code

PS: Actually if it was normal python code, I would write this part completely without staticmethods. All the information about the positions... is contained in self. The only reason why I use a staticmethod and pass in an array of floats for the positions are the aforementioned restrictions of numba.