MDAnalysis / cellgrid

MIT License
7 stars 6 forks source link

Varying precision #3

Closed richardjgowers closed 7 years ago

richardjgowers commented 8 years ago

Currently only float32 precision is supported. Ideally float64 would also be supported.

A good solution would:

kash1102 commented 7 years ago

Hi richardjgowers

I want to share my approach to fix this issue:

Any inputs on this will be appreciated. Thank you :)

richardjgowers commented 7 years ago

@kash1102 I'm not sure that really fixes anything, it might even be cleaner just to have a property which shortcuts to the array dtype, ie

@property
def dtype(self):
    return self.coordinates.dtype

With Cython the real need is to have the dtype known at compile time so you can write optimised code.

http://cython.readthedocs.io/en/latest/src/userguide/fusedtypes.html

I think what's really needed is that the maths subroutines in the library use fused types (link above). I think they didn't exist when I first ran into this issue

kash1102 commented 7 years ago

Hi richardjgowers

Sorry for the late response, I was busy with my academic projects.I have solved the issue by declaring

@property def datatype(self): return self.coordinates.dtype in core.pyx and using method name datatype of CellGrid object wherever necessary example dist = np.empty(Nreq, dtype=cg1.datatype)

and In the cgmath.pyx file simply replaced np.float_32 with fused type floating which would take care of both np.float_32 and np.float_64 values. I have tested manually by giving both np.float_32 and np.float_64 as input and also tested on test files in test directory it works perfectly fine.

Any inputs on this will be appreciated. Thanks :)

richardjgowers commented 7 years ago

Fixed in #8