-
Notifications
You must be signed in to change notification settings - Fork 50
Validate CAR coordinates and adjust area calculation #247
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
base: master
Are you sure you want to change the base?
Conversation
Added check for CAR coordinates in FITS file header and updated area calculation for 2D templates.
|
Thanks for submitting this fix, @GallegoSav! I'm concerned that this will reduce the usability of I've been looking into how to compute the pixel area and it seems the only way to deal with this properly is to compute the solid angle area for all Some other comments:
|
Introduced SpatialTemplate_2D_Healpix class for user-defined spatial templates using HealpixMap, including normalization checks and hash generation.
Removed checks for CAR coordinates and adjusted area calculation for normalization.
Removed unnecessary blank line in functions_2D.py.
Refactor SpatialTemplate_2D_Healpix class to use fits file instead of HealpixMap directly. Update parameters and methods accordingly.
|
@israelmcmc I did some changes.
It worked well for me so far. But it requires astromodels to have |
|
Thanks @GallegoSav. Maybe the warning should be conditional on the area actually being large (i.e. after a check). It could also just be part of the documentations. Otherwise people will get this warning all the time when it's actually fine. @omodei @ndilalla : are you OK with adding mhealpy as a dependency? mhealpy is healpy wrapper that I wrote. The main goal was to allow handling multi-resolution healpix maps, but it also makes map handling more object-oriented and has other features --in this particular case, for example, the automatic handling coordinate system through astropy's SkyCoord |
Removed 'ihdu' parameter from the evaluate method and its documentation.
|
@israelmcmc yeah I was thinking about the condition on the area but I don't know which value we should put |
|
Yeah, that's a fair point, since the threshold value depends on the application. My personal preference would be to add the warning just to the documentation of the function, and let the user be careful about it. I think excessive warnings are annoying and end up having the opposite effect (people end up ignoring the warnings). @omodei and @ndilalla can weight it. |
This is a quick fix for the normalization issue pointed by @israelmcmc .
With this normalization we have a correct value when passing from the ExtendedSource model to an Healpix map
However this is asking to have
CARcoordinates in the fits file likeGLON-CAR#246