Skip to content

Honor quadrature kwargs in calculate_total_face_area - #1568

Merged
Sevans711 merged 1 commit into
UXARRAY:mainfrom
vakudo:fix/total-face-area-quadrature-kwargs
Jul 14, 2026
Merged

Honor quadrature kwargs in calculate_total_face_area#1568
Sevans711 merged 1 commit into
UXARRAY:mainfrom
vakudo:fix/total-face-area-quadrature-kwargs

Conversation

@vakudo

@vakudo vakudo commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Closes #1567.

Grid.calculate_total_face_area() documents quadrature_rule, order and latitude_adjusted_area parameters, but the body ignored them and always returned np.sum(self.face_areas.values) — the cached face areas computed with the default parameters. So calling it with, for example, quadrature_rule="gaussian", order=5, latitude_adjusted_area=True gave exactly the same total as the default triangular call.

Fix

  • Keep the fast path: when the parameters are the defaults (triangular, order=4, no latitude adjustment) it still returns the cached face_areas, which also preserves the theoretical equal-area values used for HEALPix grids.
  • Otherwise it routes through _compute_face_areas(quadrature_rule, order, latitude_adjusted_area) so the requested quadrature rule, order and latitude adjustment actually take effect.

The default behavior (and the HEALPix path) is unchanged.

Tests

Added test_calculate_total_face_area_respects_quadrature_kwargs, which asserts the corrected result differs from the uncorrected one and matches the expected TRI_AREA / CORRECTED_TRI_AREA constants to 6 decimals. The existing test_face_areas_calculate_total_face_area_triangle used a decimal=3 tolerance too loose to catch this (the correction for that triangle is ~2.8e-4). The new test fails on the previous code and passes with the fix; the full test/grid/grid/test_areas.py suite passes.

Note: the "Face Area Calculations" docs page (section 2) also states the second calculation gives a slightly different area than the first — with this fix that statement becomes true when non-default quadrature settings are used.

calculate_total_face_area accepted quadrature_rule, order and
latitude_adjusted_area but ignored them, always returning the cached
default-parameter face_areas. As a result the gaussian/corrected path
produced the same total as the triangular one.

Keep the cached fast path for the default parameters (which also preserves
the equal-area values used for HEALPix grids) and route any non-default
quadrature settings through _compute_face_areas so the requested rule,
order and latitude adjustment actually take effect.
@Sevans711
Sevans711 self-requested a review July 14, 2026 12:32

@Sevans711 Sevans711 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vakudo thank you for your contribution! I also confirmed it runs, correctly gives results which actually depend on relevant kwargs, and that the new test fails on main while succeeding after your fix.

Let's merge to main!

I may open a separate issue soon related to the other parts of the face areas API. Looking into the existing methods and your fix helped me notice this. For example, uxarray now properly supports kwargs like quadrature_rule publicly in calculate_total_face_area(), while the only non-deprecated public interface for getting individual face areas (i.e., Grid.face_areas) does not support those kwargs. It seems strange to support them publicly only for the total, and not for individual faces.

@Sevans711
Sevans711 merged commit f578e30 into UXARRAY:main Jul 14, 2026
13 checks passed
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.

Face area documentation is confusing / wrong

2 participants