Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support arrays for nside #66

Open
cdeil opened this issue Oct 17, 2017 · 5 comments
Open

Support arrays for nside #66

cdeil opened this issue Oct 17, 2017 · 5 comments

Comments

@cdeil
Copy link
Member

cdeil commented Oct 17, 2017

One of the issues trying to switch Gammapy over to using astropy-healpix is that we call ang2pix with an array of nside values:
https://gist.github.com/cdeil/754b76dc7f22511a5504fbbe74dccd62#file-gistfile1-txt-L989

Minimum test case that works with healpy

>>> import healpy as hp
>>> hp.ang2pix(nside=[2, 4], theta=[0, 0], phi=[0, 0])
array([0, 0])

but doesn't work with astropy_healpix.healpy (casting to numpy arrays is missing):

>>> from astropy_healpix import healpy as hp
>>> hp.ang2pix(nside=[2, 4], theta=[0, 0], phi=[0, 0])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/deil/code/astropy-healpix/astropy_healpix/healpy.py", line 105, in ang2pix
    return lonlat_to_healpix(lon, lat, nside, order='nested' if nest else 'ring')
  File "/Users/deil/code/astropy-healpix/astropy_healpix/core.py", line 341, in lonlat_to_healpix
    nside = int(nside)
TypeError: int() argument must be a string, a bytes-like object or a number, not 'list'

and also support for array-valued nside is missing:

>>> hp.ang2pix(nside=np.array([2, 4]), theta=np.array([0, 0]), phi=np.array([0, 0]))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/deil/code/astropy-healpix/astropy_healpix/healpy.py", line 105, in ang2pix
    return lonlat_to_healpix(lon, lat, nside, order='nested' if nest else 'ring')
  File "/Users/deil/code/astropy-healpix/astropy_healpix/core.py", line 341, in lonlat_to_healpix
    nside = int(nside)
TypeError: only length-1 arrays can be converted to Python scalars

I didn't look yet how much work it would be to make this work for ang2pix or possibly also similar other functions, I'm just going through and see what's missing for Gammapy today.

@astrofrog
Copy link
Member

astrofrog commented Oct 17, 2017

In principle there's no reason we can't make the Cython wrappers take a vector nside and it would be easy. My main concern is that we need to keep it efficient for the scalar case and avoid creating an unnecessary array for nside.

Could you explain why gammapy needs this, to see if there is another way we can do this efficiently?

@astrofrog
Copy link
Member

I guess what I'm thinking is that for now it wouldn't be that inefficient to simply call the astropy-healpix routines once for each nside value, since there are only 30 possible nside values.

@astrofrog
Copy link
Member

Actually, maybe before worrying we should just try and change the Cython wrappers to use vector nside values and then if a scalar nside is passed in core.py, convert to an array. We might find the performance penalty is not actually large.

@lpsinger
Copy link
Contributor

I need this too because I use healpy for supporting NUNIQ indexing.

lpsinger added a commit to lpsinger/astropy-healpix that referenced this issue Apr 16, 2018
@cdeil
Copy link
Member Author

cdeil commented Jun 28, 2018

As commented by @lpsinger in #71 (comment) - this should still be implemented, but we should change to ufuncs first.

@cdeil cdeil modified the milestones: 0.3, 0.4 Oct 24, 2018
@cdeil cdeil modified the milestones: 0.4, 0.5 Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants