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
Add HEALPIX reprojection #27
Conversation
@keflavich Thanks for writing this! The most common use case is going to be that the user has a FITS file with a HEALPIX HDU. How about adding a second Or if it's easy to make a HEALPIX HDU from given |
@cdeil - added, in case you didn't see it. I kept the other functions because I like having different levels of access - no sense creating duplicate HDUs in memory if you've already loaded one (especially with nsides=2048 healpix, where each copy is >1 GB). @cdeil - any thoughts on generating tests for this? We shouldn't include data files, but I don't have the familiarity with healpix needed to know what would make a good test. |
Thanks for adding the extra versions! About tests: I don't think we need any real input files to test reprojection, we can always generate test images that are constant or with a checkerboard or sin pattern or something.
But I think the main thing to test is that the main use cases work, not to test how good the reprojection methods are numerically. (sorry I don't have time to work on @python-reprojection@ at the moment) |
@keflavich Do you have time to add some basic tests? |
Not at the moment, sorry - go ahead and add them if you can. |
So just to be clear, WCSLIB does include the HPX projection now - but does this do something different? |
I have no idea. I have no recollection of writing this! And it was only 6 months ago... |
I guess that this still implements reprojection, but I'm wondering whether it duplicates stuff we would want in the main reprojection routine. We should separate out the healpix-specific stuff from the reprojection itself. |
@keflavich @cdeil - do you have an example of a healpix tile we could use to try and see if it currently works with vanilla |
Hi, And the code (minus imports) is below - can anyone tell me what I'm doing wrong? Thanks! naxis1=2000 healpix_data=hp.read_map('hpMap.fits.gz',nest=True) |
@DrPhilEvans - I can look into it today. Could you share the input data with me? If so, can you send it by email to thomas.robitaille@gmail.com? |
@DrPhilEvans - and just to be clear, are you relying on the code from this pull request? (presumably that is where |
@astrofrog I think so. I'm not familiar with Github (I use svn) so I wasn't sure how to pull this - in the end I downloaded the main "reproject" project, and then added the "healpix.py" file via the "Files changed" link at the top of this page. |
@keflavich @cdeil - it would be great if we could brush up this code and add some tests. What I personally think would be nice is if for the high level interface we could just do this via the high level
and if I looked into @DrPhilEvans's issue, but it relates entirely to code in this PR (and doesn't use any of the rest of the reproject package) so @keflavich or @cdeil it would be great if you could look into it if you have some time? |
Well, I agree with you that ...edit: wait, I wrote this PR!?! It's too easy to lose track of these things. |
Thanks! |
I tried reprojecting this file:
and it looks fine (agrees with my other data). I think the problem is likely the sign flip in latitude I introduced, which was necessary to get the reprojected image to agree with Galactic coordinates and is therefore correct for one coordinate system... but apparently not for the other? I'll investigate a little further. |
@keflavich Thanks. |
Well, I've uncovered some bugs, and when I tried to transform to a tangent projection, it just crashed on me:
so this is already getting me pretty far out of my depth. |
@keflavich I think |
I keep getting emails asking for this functionality (because there used to be this old function that never quite worked in Gammapy). I once again don't have time to work on this and am not very good with HEALPIX in the first place. Some more references that might be useful: |
Sounds like something we could sprint on next week? |
Definitely! |
What is the exact call that is crashing? I'm going to try to debug it... |
>>> healpix_isnested = fits.getheader(healpix_filename,ext=1)['ORDERING'] == 'NESTED' | ||
>>> reference_image = fits.open('reference_image.fits')[0] | ||
>>> reprojected_data = healpix_to_image(healpix_data, reference_image, healpix_system, nest=healpix_isnested) | ||
>>> fits.writeto('new_image.fits', reprojected_data, reference_image.header) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem here: by default, hp.read_map
reorders the pixels to RING
order. I'll make a pull request to fix this...
@lpsinger - if I use the incorrect sign for lat, I get a crash. Otherwise, it works. However, I haven't tested for the RA/Dec one reported above. This is my incomplete / WIP test code (note that it is not a functioning test and it is missing data):
|
Is reproject set up to have its doctests run as part of |
Yes, I think reproject runs doctests... but I can't prove it... |
Can you provide me with a reference header? |
No, it doesn't... I asked on the Astropy list how to turn this on. Anyway, I've gotten healpix_to_image working as of 38ff431. Still need to fix doctests and turn them on. I am using this HEALPix test file: Here's a plot of the original HEALPix image: And here is my test script:
And here a screen shot of the output image in DS9: |
For the doctests, I think there's a doctest_plus option in |
@astrofrog - it doesn't run atm, I can open a PR for it tomorrow. |
@bsipocz - just to check, do you mean that you will open a PR to edit |
@lpsinger - thanks for working on this! Could you open a PR to @keflavich's branch so that we can include the commits here? |
This is a minor point, but when I last used the reproject option to make On 25/04/15 10:40, Thomas Robitaille wrote:
Phil Evans, Tel: +44 (0)116 252 5059 http://www.star.le.ac.uk/~pae9 Follow me as a Swift scientist on Twitter: @swift_phil |
@DrPhilEvans, did you try my branch? I'm not getting warning messages. |
Ah, no probably not. I got a bit lost navigating the different trees :S On 27/04/15 15:59, Leo Singer wrote:
Phil Evans, Tel: +44 (0)116 252 5059 http://www.star.le.ac.uk/~pae9 Follow me as a Swift scientist on Twitter: @swift_phil |
@lpsinger - just to let you know, when you open a pull request to @keflavich's repo, this is the repo to open a PR to: https://github.com/keflavich/python-reprojection (the repo here changed name after @keflavich forked it) |
|
…ix, healpix_reproject_file Note that there will be some issues with round-trip projection (HEALPix->WCS->HEALPix) if some of the pixels lie precisely on the WCS map boundary.
Working and tested implementation of healpix_to_image, image_to_healpix, and healpix_reproject_file
@keflavich - can you change line 96 in
to
? If you have no time on this you can also add me (temporarily) as a committer on your |
FYI: possibly related to using |
I'm going to go ahead and merge this now to take the burden off @keflavich but @lpsinger, I have some questions I'll raise in a separate issue. |
Tool to convert HEALPIX images to Galactic or ICRS coordinates, based tightly on @cdeil's function in gammapy. See healpy/healpy#129.