Skip to content

Conversation

@GallegoSav
Copy link
Contributor

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

from mhealpy import HealpixMap


# Define healpix map :
skymap = HealpixMap(nside = 16, scheme = "ring", dtype = float, coordsys='G')
coords1 = skymap.pix2skycoord(range(skymap.npix))
pix_area = skymap.pixarea().value

#src1 being a Extendedsource model using SpatialTemplate_2D for the spatial shape
skymap[:] = src1.spatial_shape(coords1.l.deg, coords1.b.deg) 

# Check normalization:
print("summed map: " + str(np.sum(skymap)*pix_area))

#summed map: 0.9925528860013819 
#before this fix it was around 0.82

However this is asking to have CAR coordinates in the fits file like GLON-CAR

#246

Added check for CAR coordinates in FITS file header and updated area calculation for 2D templates.
@ndilalla ndilalla requested a review from israelmcmc February 3, 2026 22:41
@israelmcmc
Copy link
Contributor

Thanks for submitting this fix, @GallegoSav! I'm concerned that this will reduce the usability of SpatialTemplate_2D to only the CAR projection when there are many more, and various of them are frequently use. Before this change, SpatialTemplate_2D supported all of them, with the limitation of having a correct normalization only for relatively small images. This is one of the reasons why the tests are failing (there's a TAN in the tests, as least)

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 pixel_to_world_values using the edges of the corresponding spherical rectangles. We can either do that, or restrict SpatialTemplate_2D to relatively small regions and use instead HEALPix for whole-sky models.

Some other comments:

  • Some tests are also failing due to linting (style checks) e.g.
Run # stop the build if there are Python syntax errors or undefined names
activating heasoft in /Users/runner/miniconda3/envs/test_env/heasoft
./astromodels/functions/functions_2D.py:727:13: E265 block comment should start with '# '
./astromodels/functions/functions_2D.py:729:42: E225 missing whitespace around operator
./astromodels/functions/functions_2D.py:730:89: E501 line too long (139 > 88 characters)
./astromodels/functions/functions_2D.py:731:1: W293 blank line contains whitespace
./astromodels/functions/functions_2D.py:751:1: W293 blank line contains whitespace
./astromodels/functions/functions_2D.py:752:13: E265 block comment should start with '# '
./astromodels/functions/functions_2D.py:753:13: E265 block comment should start with '# '
./astromodels/functions/functions_2D.py:755:1: W293 blank line contains whitespace
./astromodels/functions/functions_2D.py:758:63: E262 inline comment should start with '# '
  • I think this expression d_lon = np.sqrt(area_deg2).to(u.rad).value should take into account the latitude of the center pixel (where the area_deg2 is computed), which is not necessarily at the equator.

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.
@GallegoSav
Copy link
Contributor Author

GallegoSav commented Feb 6, 2026

@israelmcmc I did some changes.

  1. I put back the SpatialTemplate_2D class as it was and just added a warning to warn the user to use the new class if you have a big region model.
  2. I added a new class SpatialTemplate_2D_Healpix which takes as input a fits file with a healpix map

It worked well for me so far. But it requires astromodels to have mhealpy installed. It is not very clear to me where the package dependencies are define

@israelmcmc
Copy link
Contributor

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.
@GallegoSav
Copy link
Contributor Author

@israelmcmc yeah I was thinking about the condition on the area but I don't know which value we should put

@israelmcmc
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants