search

astropy / astropy-healpix

BSD-licensed HEALPix for Astropy - maintained by @astrofrog and @lpsinger
https://astropy-healpix.readthedocs.io
BSD 3-Clause "New" or "Revised" License
53 stars 22 forks source link

Convert C code to use only int64_t internally #64

Open astrofrog opened 7 years ago

astrofrog commented 7 years ago

At the moment there is a lot of typecasting going on in the C code between int and int64_t. As suggested by @cdeil it would be a lot simpler if we just used int64_t throughout, although there may be a few remaining cases where this is really not needed.

lpsinger commented 6 years ago

Hmm, what about providing both 32-bit and 64-bit ufuncs?

astrofrog commented 6 years ago

Just to clarify, the issue is that internally for calculations related to indices, 32-bit ints are usually not enough - and the C code currently uses 32-bit ints in places. I think I've switched all the ones that matter to 64-bit, but this issue was just suggesting we could just convert all of them. This is not necessarily related to the int type of the indices passed to the cython functions/ufuncs.

lpsinger commented 6 years ago

But I don't think that you can represent Numpy arrays that require 64-bit indices on a 32-bit machine, can you?

astrofrog commented 6 years ago

@lpsinger - that's true - but just to be clear you can still request for example a 64-bit HEALPix index from e.g. lonlat_to_healpix on a 32-bit machine (and we do test that that works in the CI). So you don't need a 32-bit version of that function. But I agree that I think if you wanted to actually use that index in an array it would be an issue.

astrofrog commented 6 years ago

Just to be clear the original issue here was more about the underlying C library which is not super consistent in when it uses 32- vs 64-bit integers, including on a 64-bit platform (that part of the code knows nothing about Numpy arrays). I think you are talking about the cython or ufunc layer?