Skip to content

Allow SCRIP reader to respect units w/r/t radians - #1433

Merged
rajeeja merged 6 commits into
mainfrom
fix-scrip-radians-units
Jul 14, 2026
Merged

Allow SCRIP reader to respect units w/r/t radians#1433
rajeeja merged 6 commits into
mainfrom
fix-scrip-radians-units

Conversation

@zarzycki

@zarzyckizarzycki commented Dec 30, 2025•
edited by Sevans711
Loading

Copy link
Copy Markdown
Contributor

Overview

  • Currently, the SCRIP reader does not respect units attached to relevant lat/lon vars. That is, it assumes degrees. For 90% of grids, this is probably the case. MPAS, by default, writes SCRIP files internally in radians, which are easily convertible using NCO or other external scripts, but it breaks uxarray currently (well, uxarray runs fine since |radians| < |degrees| but then projects fields in a very small location near 0, 0).
  • This just adds a simple check to see if "radians" (or "radians-adjacent") units are attached to the grid_XX_lat/lon vars on the SCRIP nc file and, if so, converts upon loading.

PR Checklist

General

  • An issue is linked created and linked
  • Add appropriate labels
  • Filled out Overview and Expected Usage (if applicable) sections

Testing

  • Adequate tests are created if there is new functionality
  • Tests cover all possible logical paths in your function
  • Tests are not too basic (such as simply calling a function and nothing else)

Documentation

  • Docstrings have been added to all new functions
  • [N/A] Docstrings have updated with any function changes
  • Internal functions have a preceding underscore (_) and have been added to docs/internal_api/index.rst
  • [N/A] User functions have been added to docs/user_api/index.rst

@zarzycki
zarzycki marked this pull request as draft December 30, 2025 17:25
@Sevans711

Sevans711 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Hi @zarzycki, thank you for drafting this PR! It has been a while, so I wanted to check, can you confirm whether this still needs to be fixed, in the latest version of uxarray?

If yes, it would be extremely helpful if you might be able to provide a tiny example (~1 MB or less would be ideal) where the issue occurs.

Then, the remaining steps would be:

  1. make an issue report (https://github.com/UXARRAY/uxarray/issues) for this issue.
  2. add the tiny example file, somewhere in uxarray/test/meshfiles/scrip.
  3. make a test (in uxarray/test/io/test_scrip.py) to check units are handled properly after your fix.
  4. merge main into this branch (via the "Update branch" button) and make sure all tests pass.
  5. someone else reviews the changes and merges the fix into main.

To make things as easy as possible, if you can confirm this is still an issue and can provide a tiny example file here, I'd be happy to take over any/all of steps 1 through 4 if you want, and then hopefully we could get this fix included in the next uxarray release!

@Sevans711Sevans711 added the bug Something isn't working label Jul 10, 2026
@zarzycki

zarzycki commented Jul 13, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks @Sevans711. I can confirm the bug is still present in uxarray 2026.6.0.

A SCRIP file with coordinates stored in radians is read without conversion; e.g. face_lon returns [0.17, 0.35] instead of [10.0, 20.0].

I've opened issue #1564 and pushed a tiny (synthetic) SCRIP file (test/meshfiles/scrip/scrip_radians.nc) along with a test (test_scrip_radians_units in test/io/test_scrip.py) that reproduces the failure on unpatched uxarray and passes with this fix. I remain a Github novice-ish when issuing PRs for a project I am not running point on, so please let me know if other things are needed from me (Claude helped here :)

@Sevans711Sevans711 self-assigned this Jul 13, 2026
Sevans711and others added 2 commits July 13, 2026 15:09
@Sevans711
minor formatting changes for scrip radians fix
b702f79
Moves meshfiles/scrip/scrip_radians.nc to meshfiles/scrip/scrip_radians/scrip_radians_grid.nc to match style of other meshfiles naming schemes.
Renames the new _scrip._convert_to_degrees() to _scrip._values_in_degrees(). It doesn't always convert; and it also does more than just converting, because it gives numpy array from DataArray. Clarified docstring.
ran the following, so it should now pass ruff checks: pre-commit run --all-files
@Sevans711
Merge branch 'main' into fix-scrip-radians-units
4024592
@Sevans711
Sevans711 marked this pull request as ready for review July 13, 2026 19:13
@Sevans711

Sevans711 commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

The small test case looks good, I was also able to confirm it fails on main but succeeds after your fix. Thank you @zarzycki for all your efforts here! I made some minor formatting/style changes but this looks good to me now and I think it is ready to merge to main.

Pinging @rajeeja or @erogluorhan to review if you get a chance! Additionally, I notice now that the PR default form (see original message) includes the task "Internal functions have a preceding underscore (_) and have been added to docs/internal_api/index.rst." Can you confirm whether this is necessary? I don't actually see the docs/internal_api/index.rst file, maybe it has been renamed or removed?

rajeeja
rajeeja approved these changes Jul 14, 2026
View reviewed changes

@rajeejarajeeja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified locally — radians convert correctly, degrees files unaffected, SCRIP tests pass. Thanks @zarzycki.

@rajeeja
rajeeja merged commit 60ea985 into mainJul 14, 2026
13 checks passed
@rajeeja
rajeeja deleted the fix-scrip-radians-units branch July 14, 2026 17:37
@Sevans711Sevans711 mentioned this pull request Jul 14, 2026
Closed
rajeeja added a commit that referenced this pull request Jul 16, 2026
@cmdupuis3@vakudo
@Sevans711@rajeeja@erogluorhan@zarzycki@dependabot@pre-commit-ci
Cmd/accusphere (#1579)
465fd4c
* Honor quadrature kwargs in calculate_total_face_area
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.
* Restore matplotlib backend after HoloViews matplotlib plot (#1538)
* Restore matplotlib backend after HoloViews matplotlib plot
plot(backend='matplotlib') calls hv.extension('matplotlib'), which switches
the active matplotlib backend and clobbers the IPython inline display hook,
silently breaking subsequent native matplotlib/xarray .plot() calls. Restore
the original matplotlib backend right after the HoloViews extension switch;
HoloViews objects still display via Store.current_backend, so this is safe.
Closes#1537
* Address review: capture backend at switch, accurate docstring, effective test
* Reconfigure IPython inline display hook when restoring backend
mpl.use() restores the matplotlib backend name but does not re-register
IPython's inline display integration that hv.extension('matplotlib')
clobbers. In a Jupyter kernel without an explicit %matplotlib inline,
native matplotlib/xarray .plot() calls after a uxarray matplotlib plot
still failed to render. Re-run configure_inline_support when the restored
backend is inline so the display hook is reinstated. See #1537.
* Restore matplotlib backend via IPython shell reactivation to fix inline display
---------
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
* Allow SCRIP reader to respect units w/r/t radians (#1433)
* Allow SCRIP reader to respect units wrt to radians
* Add test file and unit test for SCRIP radian coordinate handling
* minor formatting changes for scrip radians fix
Moves meshfiles/scrip/scrip_radians.nc to meshfiles/scrip/scrip_radians/scrip_radians_grid.nc to match style of other meshfiles naming schemes.
Renames the new _scrip._convert_to_degrees() to _scrip._values_in_degrees(). It doesn't always convert; and it also does more than just converting, because it gives numpy array from DataArray. Clarified docstring.
ran the following, so it should now pass ruff checks: pre-commit run --all-files
---------
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Rajeev Jain <rajeeja@gmail.com>
* Support Python 3.14 and upgrade YAC to v3.18 (with DNN remapping) (#1563)
* Upgrade YAC to v3.18, expose DNN remapping, drop pathlib backport
- upgrade YAC CI to v3.18.0 on Python 3.14 (numba>=0.63, py3.14 classifier,
cython>=3.1 via conda for the bindings build)
- fix add_average: reduction_type -> weight_type (renamed in YAC v3.18)
- expose distance-nearest-neighbour (yac_method='dnn', new in YAC v3.15) + test
- remove the obsolete 'pathlib' backport dependency: it shadows the stdlib and
breaks tools that import pathlib on Python 3.10+ (broke YAC's Cython build)
* Reference #1561 in numba version pin comment
---------
Co-authored-by: Christopher Dupuis <45972964+cmdupuis3@users.noreply.github.com>
* Bump actions/download-artifact in the actions group across 1 directory (#1572)
Bumps the actions group with 1 update in the / directory: [actions/download-artifact](https://github.com/actions/download-artifact).
Updates `actions/download-artifact` from 7 to 8
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v7...v8)
---
updated-dependencies:
- dependency-name: actions/download-artifact
dependency-version: '8'
dependency-type: direct:production
update-type: version-update:semver-major
dependency-group: actions
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* [pre-commit.ci] pre-commit autoupdate (#1565)
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.15.20 → v0.15.21](astral-sh/ruff-pre-commit@v0.15.20...v0.15.21)
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Accusphere: gca_gca benchmarks and better arc sampling
* Accusphere: fix sum_of_squares accuracy bug + tests
* Accusphere: numba fma refactor and fix validation tautology
* Accusphere: scalarized _counts_as_crossing
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: vakudo <127726617+vakudo@users.noreply.github.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Rajeev Jain <rajeeja@gmail.com>
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
Co-authored-by: zarzycki <colin.zarzycki@gmail.com>
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
rajeeja added a commit that referenced this pull request Jul 28, 2026
@rajeeja@hongyuchen1030
@cmdupuis3@vakudo@Sevans711@erogluorhan@zarzycki@dependabot@pre-commit-ci
EFT-based compensated arithmetic for spherical geometry (GCA intersec…
73101af
…tion, point-in-face, face bounds) (#1513)
* Port AccuSphGeom EFT algorithms and add spherical geometry user guide
Closes#1509
* Fix pre-commit: remove unused imports and variable
* Fix pre-commit: split semicolons in notebook cells
* Address Hongyu's review: port AccuSphGeom compensated arithmetic, rewrite intersections, add 241 baseline testsgit status! - most came from accusphere
* Fix RTD: remove RST-invalid numbered list from gca_const_lat_intersection docstring
* Fix RTD notebook kernel metadata
* Address AccuSphGeom review feedback
* Separate intersection kernels into three layers: numerical core, status/mask, dispatcher
* Fix NaN/Inf propagation in intersection kernels for denom=0 and planar_sq<0
* Add geometry kernel benchmarks and to_raster auto-extent test
- benchmarks/geometry_kernels.py: ASV micro-benchmarks for all three
layers of the EFT intersection stack (_accux_gca, _try_gca_gca_intersection,
gca_gca_intersection, _accux_constlat, _try_gca_const_lat_intersection,
gca_const_lat_intersection) plus EFT primitives and point-in-polygon;
all functions warmed before timing so results reflect steady-state cost
- test/test_plot.py: add test_to_raster_auto_extent verifying that the
axis limits change and the raster contains finite data
* Address AccuSphGeom review: precision wording, dead code removal, correctness fixes
Review comments addressed:
- Remove "near-double precision" / "sufficient" overclaims; say "roughly twice
as accurate" and note the robustness tier boundary clearly
- Explain _lon_bounds_from_vertices is required for UXarray antimeridian
encoding and cannot be removed
- Add block comment before _no_extreme functions clarifying they are
pre-existing edge screeners unrelated to the EFT stack
- Document SoS as explicit future work in _point_in_polygon_sphere docstring
- L2 pos_fin/neg_fin: replace ternary with int(); exploit neg=-pos symmetry
- Label computation: drop dead local*0 term, use integer mask arithmetic
- Remove vertex-lat snap from bounds: _face_location_info already captures
interior arc extrema accurately via the compensated kernel
- _ON_MINOR_ARC_TOL: document intentional 1e-10 vs C++ 1e-8 divergence
Bug fixes:
- on_minor_arc: add antipodal-endpoint guard; a x b = 0 for antipodal inputs
so every point on the great circle passes the collinearity test (false pos)
- bounds.py: replace mask arithmetic use_ext*z_ext + (1-use_ext)*z_edge with
plain if/else; 0*NaN = NaN propagates when norm=0, if/else does not
- _point_in_polygon_sphere: ray-nudge now restarts the loop from i=0 so all
edges are counted with the same ray (mid-loop nudge corrupted crossing parity)
Cleanup:
- Remove _flip_sign, _SIGN_NEG, _SIGN_POS, _SIGN_ZERO dead code from
point_in_face.py; inline literals in _counts_as_crossing
- Remove _SNAP_TOL_DEG constant and snap_tol_deg parameter throughout bounds.py
- Notebook: fix Grid.get_point_on_face -> get_faces_containing_point; remove
incorrect geometry.py row from Section 4 table; add accucross_pair and
acc_sqrt_re to Section 2 building-blocks table
* Pin tornado<6.5.7 to fix Windows CI; remove dead shim; fix notebook API name
- ci/environment.yml: pin tornado<6.5.7 to avoid ssl.SSLError in panel 1.9.3
on Python 3.11 Windows (conda-forge regression, 2026-06-10)
- intersections.py: remove _gca_gca_intersection_cartesian shim (dead code);
add comment explaining _snap_const_lat_endpoint snap_sq constant
- test_intersections.py: update 4 call sites to use gca_gca_intersection directly
- spherical-geometry-accuracy.ipynb: fix stale Grid.get_point_on_face ->
Grid.get_faces_containing_point (2 occurrences)
* Trim computing.py docstring; drop inline=always from L1 kernels
* Revert tornado pin; root cause is openssl 3.6.3 on Windows
* Port AccuSphGeom EFT algorithms and add spherical geometry user guide
Closes#1509
* Fix pre-commit: remove unused imports and variable
* Fix pre-commit: split semicolons in notebook cells
* Address Hongyu's review: port AccuSphGeom compensated arithmetic, rewrite intersections, add 241 baseline testsgit status! - most came from accusphere
* Fix RTD: remove RST-invalid numbered list from gca_const_lat_intersection docstring
* Fix RTD notebook kernel metadata
* Address AccuSphGeom review feedback
* Separate intersection kernels into three layers: numerical core, status/mask, dispatcher
* Fix NaN/Inf propagation in intersection kernels for denom=0 and planar_sq<0
* computing: use FMA two_prod where available (portable Veltkamp fallback)
Add an LLVM fma intrinsic and route two_prod through a single fused
multiply-add for its error term on hardware that supports it, selected at
import time and validated to be bit-exact against the Veltkamp split. Falls
back to the portable Veltkamp form otherwise, so there is no hard FMA
dependency.
The FMA path is ~2x faster in the compensated geometry kernels (each
two_prod drops from ~17 flops to one FMADD) and is numerically identical:
all 241 AccuSphGeom baseline cases pass unchanged.
* intersections: add allocation-free scalar const-lat L1 kernel
Add _accux_constlat_scalar, which takes the arc endpoints as six scalars and
returns the candidate coordinates as scalars instead of two np.empty(3)
arrays. _accux_constlat now wraps it so the array API is unchanged.
Returning scalars lets Numba keep the candidates in registers, so a batch
loop over many edges does no per-point heap allocation. On a 16M-point
const-lat sweep this is ~2.7x faster than the array-returning path and drops
the AccuX/FP64 cost ratio from ~19x to ~7x. Bit-identical results; all 241
AccuSphGeom baseline cases pass.
* style: apply ruff formatting to computing.py
* Clean up spherical geometry review items
* Scalarize constant-latitude intersection dispatcher to remove per-call heap allocations
* Import benchmark kernels inside setup to avoid asv collection failures
Move the uxarray kernel imports from module level into each benchmark
class's setup, so a stale or mismatched environment build only errors the
affected benchmark instead of aborting collection of every benchmark in
the directory. Thanks @cmdupuis3 for catching the asv import failure.
* Cmd/accusphere (#1579)
* Honor quadrature kwargs in calculate_total_face_area
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.
* Restore matplotlib backend after HoloViews matplotlib plot (#1538)
* Restore matplotlib backend after HoloViews matplotlib plot
plot(backend='matplotlib') calls hv.extension('matplotlib'), which switches
the active matplotlib backend and clobbers the IPython inline display hook,
silently breaking subsequent native matplotlib/xarray .plot() calls. Restore
the original matplotlib backend right after the HoloViews extension switch;
HoloViews objects still display via Store.current_backend, so this is safe.
Closes#1537
* Address review: capture backend at switch, accurate docstring, effective test
* Reconfigure IPython inline display hook when restoring backend
mpl.use() restores the matplotlib backend name but does not re-register
IPython's inline display integration that hv.extension('matplotlib')
clobbers. In a Jupyter kernel without an explicit %matplotlib inline,
native matplotlib/xarray .plot() calls after a uxarray matplotlib plot
still failed to render. Re-run configure_inline_support when the restored
backend is inline so the display hook is reinstated. See #1537.
* Restore matplotlib backend via IPython shell reactivation to fix inline display
---------
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
* Allow SCRIP reader to respect units w/r/t radians (#1433)
* Allow SCRIP reader to respect units wrt to radians
* Add test file and unit test for SCRIP radian coordinate handling
* minor formatting changes for scrip radians fix
Moves meshfiles/scrip/scrip_radians.nc to meshfiles/scrip/scrip_radians/scrip_radians_grid.nc to match style of other meshfiles naming schemes.
Renames the new _scrip._convert_to_degrees() to _scrip._values_in_degrees(). It doesn't always convert; and it also does more than just converting, because it gives numpy array from DataArray. Clarified docstring.
ran the following, so it should now pass ruff checks: pre-commit run --all-files
---------
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Rajeev Jain <rajeeja@gmail.com>
* Support Python 3.14 and upgrade YAC to v3.18 (with DNN remapping) (#1563)
* Upgrade YAC to v3.18, expose DNN remapping, drop pathlib backport
- upgrade YAC CI to v3.18.0 on Python 3.14 (numba>=0.63, py3.14 classifier,
cython>=3.1 via conda for the bindings build)
- fix add_average: reduction_type -> weight_type (renamed in YAC v3.18)
- expose distance-nearest-neighbour (yac_method='dnn', new in YAC v3.15) + test
- remove the obsolete 'pathlib' backport dependency: it shadows the stdlib and
breaks tools that import pathlib on Python 3.10+ (broke YAC's Cython build)
* Reference #1561 in numba version pin comment
---------
Co-authored-by: Christopher Dupuis <45972964+cmdupuis3@users.noreply.github.com>
* Bump actions/download-artifact in the actions group across 1 directory (#1572)
Bumps the actions group with 1 update in the / directory: [actions/download-artifact](https://github.com/actions/download-artifact).
Updates `actions/download-artifact` from 7 to 8
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v7...v8)
---
updated-dependencies:
- dependency-name: actions/download-artifact
dependency-version: '8'
dependency-type: direct:production
update-type: version-update:semver-major
dependency-group: actions
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* [pre-commit.ci] pre-commit autoupdate (#1565)
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.15.20 → v0.15.21](astral-sh/ruff-pre-commit@v0.15.20...v0.15.21)
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Accusphere: gca_gca benchmarks and better arc sampling
* Accusphere: fix sum_of_squares accuracy bug + tests
* Accusphere: numba fma refactor and fix validation tautology
* Accusphere: scalarized _counts_as_crossing
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: vakudo <127726617+vakudo@users.noreply.github.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Rajeev Jain <rajeeja@gmail.com>
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
Co-authored-by: zarzycki <colin.zarzycki@gmail.com>
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Add thread-scaling benchmark for FP64 vs AccuX constlat dispatcher
* Cap thread-scaling sweep at performance cores to avoid E-core artifact
* Fix thread-scaling plot ticks to show only 1,2,4,8
* o Fix pre-commit
* o doc fixes
* o Remove benchmark image from repo
* Split out point-in-face and lat-lon bounds to a separate PR
* o computing: match AccuSphGeom acc_sqrt_re exactly (residual order, branch-free)
* o intersections: compensated norm in AccuXGCA kernel, branch-free mask validity per AccuSphGeom
* o arcs: branch-free on_minor_arc int mask at tol 1e-8 matching AccuSphGeom
* o benchmarks: drop point-in-polygon kernel bench, moved to point-in-face PR
* o benchmarks: drop thread-scaling constlat, moved to separate scaling issue
* o docs: move spherical-geometry notebook out, depends on split-out point-in-face
* o docs: drop spherical-geometry notebook from user-guide toctree
* o test_plot: drop to_raster_auto_extent, unrelated to AccuXGCA/AccuXConstLat scope
* o fix on_minor_arc antipodal false positive
* o add on_minor_arc degeneracy regression tests
* fix diff_of_products addition grouping to match AccuSphGeom eft.hpp
* o intersections: collapse pos/neg-valid branches via XOR in gca_const_lat_intersection
* arcs: keep normal hi/lo split through _normal_dot_value via compensated dot
* arcs: fix comments overclaiming branch-free form and degeneracy-check robustness
* arcs: stop claiming on_minor_arc mirrors AccuSphGeom, it's an approximation
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Hongyu Chen <hyvchen@ucdavis.edu>
Co-authored-by: Christopher Dupuis <45972964+cmdupuis3@users.noreply.github.com>
Co-authored-by: vakudo <127726617+vakudo@users.noreply.github.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
Co-authored-by: zarzycki <colin.zarzycki@gmail.com>
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
erogluorhan added a commit that referenced this pull request Aug 18, 2026
@cmdupuis3@rajeeja
@hongyuchen1030@vakudo@Sevans711@erogluorhan@zarzycki@dependabot@pre-commit-ci
Better chunking strategies for constlat intersections and zonal routi…
23b8d5c
…nes. (#1624)
* Port AccuSphGeom EFT algorithms and add spherical geometry user guide
Closes#1509
* Fix pre-commit: remove unused imports and variable
* Fix pre-commit: split semicolons in notebook cells
* Address Hongyu's review: port AccuSphGeom compensated arithmetic, rewrite intersections, add 241 baseline testsgit status! - most came from accusphere
* Fix RTD: remove RST-invalid numbered list from gca_const_lat_intersection docstring
* Fix RTD notebook kernel metadata
* Address AccuSphGeom review feedback
* Separate intersection kernels into three layers: numerical core, status/mask, dispatcher
* Fix NaN/Inf propagation in intersection kernels for denom=0 and planar_sq<0
* Add geometry kernel benchmarks and to_raster auto-extent test
- benchmarks/geometry_kernels.py: ASV micro-benchmarks for all three
layers of the EFT intersection stack (_accux_gca, _try_gca_gca_intersection,
gca_gca_intersection, _accux_constlat, _try_gca_const_lat_intersection,
gca_const_lat_intersection) plus EFT primitives and point-in-polygon;
all functions warmed before timing so results reflect steady-state cost
- test/test_plot.py: add test_to_raster_auto_extent verifying that the
axis limits change and the raster contains finite data
* Address AccuSphGeom review: precision wording, dead code removal, correctness fixes
Review comments addressed:
- Remove "near-double precision" / "sufficient" overclaims; say "roughly twice
as accurate" and note the robustness tier boundary clearly
- Explain _lon_bounds_from_vertices is required for UXarray antimeridian
encoding and cannot be removed
- Add block comment before _no_extreme functions clarifying they are
pre-existing edge screeners unrelated to the EFT stack
- Document SoS as explicit future work in _point_in_polygon_sphere docstring
- L2 pos_fin/neg_fin: replace ternary with int(); exploit neg=-pos symmetry
- Label computation: drop dead local*0 term, use integer mask arithmetic
- Remove vertex-lat snap from bounds: _face_location_info already captures
interior arc extrema accurately via the compensated kernel
- _ON_MINOR_ARC_TOL: document intentional 1e-10 vs C++ 1e-8 divergence
Bug fixes:
- on_minor_arc: add antipodal-endpoint guard; a x b = 0 for antipodal inputs
so every point on the great circle passes the collinearity test (false pos)
- bounds.py: replace mask arithmetic use_ext*z_ext + (1-use_ext)*z_edge with
plain if/else; 0*NaN = NaN propagates when norm=0, if/else does not
- _point_in_polygon_sphere: ray-nudge now restarts the loop from i=0 so all
edges are counted with the same ray (mid-loop nudge corrupted crossing parity)
Cleanup:
- Remove _flip_sign, _SIGN_NEG, _SIGN_POS, _SIGN_ZERO dead code from
point_in_face.py; inline literals in _counts_as_crossing
- Remove _SNAP_TOL_DEG constant and snap_tol_deg parameter throughout bounds.py
- Notebook: fix Grid.get_point_on_face -> get_faces_containing_point; remove
incorrect geometry.py row from Section 4 table; add accucross_pair and
acc_sqrt_re to Section 2 building-blocks table
* Pin tornado<6.5.7 to fix Windows CI; remove dead shim; fix notebook API name
- ci/environment.yml: pin tornado<6.5.7 to avoid ssl.SSLError in panel 1.9.3
on Python 3.11 Windows (conda-forge regression, 2026-06-10)
- intersections.py: remove _gca_gca_intersection_cartesian shim (dead code);
add comment explaining _snap_const_lat_endpoint snap_sq constant
- test_intersections.py: update 4 call sites to use gca_gca_intersection directly
- spherical-geometry-accuracy.ipynb: fix stale Grid.get_point_on_face ->
Grid.get_faces_containing_point (2 occurrences)
* Trim computing.py docstring; drop inline=always from L1 kernels
* Revert tornado pin; root cause is openssl 3.6.3 on Windows
* Port AccuSphGeom EFT algorithms and add spherical geometry user guide
Closes#1509
* Fix pre-commit: remove unused imports and variable
* Fix pre-commit: split semicolons in notebook cells
* Address Hongyu's review: port AccuSphGeom compensated arithmetic, rewrite intersections, add 241 baseline testsgit status! - most came from accusphere
* Fix RTD: remove RST-invalid numbered list from gca_const_lat_intersection docstring
* Fix RTD notebook kernel metadata
* Address AccuSphGeom review feedback
* Separate intersection kernels into three layers: numerical core, status/mask, dispatcher
* Fix NaN/Inf propagation in intersection kernels for denom=0 and planar_sq<0
* computing: use FMA two_prod where available (portable Veltkamp fallback)
Add an LLVM fma intrinsic and route two_prod through a single fused
multiply-add for its error term on hardware that supports it, selected at
import time and validated to be bit-exact against the Veltkamp split. Falls
back to the portable Veltkamp form otherwise, so there is no hard FMA
dependency.
The FMA path is ~2x faster in the compensated geometry kernels (each
two_prod drops from ~17 flops to one FMADD) and is numerically identical:
all 241 AccuSphGeom baseline cases pass unchanged.
* intersections: add allocation-free scalar const-lat L1 kernel
Add _accux_constlat_scalar, which takes the arc endpoints as six scalars and
returns the candidate coordinates as scalars instead of two np.empty(3)
arrays. _accux_constlat now wraps it so the array API is unchanged.
Returning scalars lets Numba keep the candidates in registers, so a batch
loop over many edges does no per-point heap allocation. On a 16M-point
const-lat sweep this is ~2.7x faster than the array-returning path and drops
the AccuX/FP64 cost ratio from ~19x to ~7x. Bit-identical results; all 241
AccuSphGeom baseline cases pass.
* style: apply ruff formatting to computing.py
* Clean up spherical geometry review items
* Scalarize constant-latitude intersection dispatcher to remove per-call heap allocations
* Import benchmark kernels inside setup to avoid asv collection failures
Move the uxarray kernel imports from module level into each benchmark
class's setup, so a stale or mismatched environment build only errors the
affected benchmark instead of aborting collection of every benchmark in
the directory. Thanks @cmdupuis3 for catching the asv import failure.
* Cmd/accusphere (#1579)
* Honor quadrature kwargs in calculate_total_face_area
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.
* Restore matplotlib backend after HoloViews matplotlib plot (#1538)
* Restore matplotlib backend after HoloViews matplotlib plot
plot(backend='matplotlib') calls hv.extension('matplotlib'), which switches
the active matplotlib backend and clobbers the IPython inline display hook,
silently breaking subsequent native matplotlib/xarray .plot() calls. Restore
the original matplotlib backend right after the HoloViews extension switch;
HoloViews objects still display via Store.current_backend, so this is safe.
Closes#1537
* Address review: capture backend at switch, accurate docstring, effective test
* Reconfigure IPython inline display hook when restoring backend
mpl.use() restores the matplotlib backend name but does not re-register
IPython's inline display integration that hv.extension('matplotlib')
clobbers. In a Jupyter kernel without an explicit %matplotlib inline,
native matplotlib/xarray .plot() calls after a uxarray matplotlib plot
still failed to render. Re-run configure_inline_support when the restored
backend is inline so the display hook is reinstated. See #1537.
* Restore matplotlib backend via IPython shell reactivation to fix inline display
---------
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
* Allow SCRIP reader to respect units w/r/t radians (#1433)
* Allow SCRIP reader to respect units wrt to radians
* Add test file and unit test for SCRIP radian coordinate handling
* minor formatting changes for scrip radians fix
Moves meshfiles/scrip/scrip_radians.nc to meshfiles/scrip/scrip_radians/scrip_radians_grid.nc to match style of other meshfiles naming schemes.
Renames the new _scrip._convert_to_degrees() to _scrip._values_in_degrees(). It doesn't always convert; and it also does more than just converting, because it gives numpy array from DataArray. Clarified docstring.
ran the following, so it should now pass ruff checks: pre-commit run --all-files
---------
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Rajeev Jain <rajeeja@gmail.com>
* Support Python 3.14 and upgrade YAC to v3.18 (with DNN remapping) (#1563)
* Upgrade YAC to v3.18, expose DNN remapping, drop pathlib backport
- upgrade YAC CI to v3.18.0 on Python 3.14 (numba>=0.63, py3.14 classifier,
cython>=3.1 via conda for the bindings build)
- fix add_average: reduction_type -> weight_type (renamed in YAC v3.18)
- expose distance-nearest-neighbour (yac_method='dnn', new in YAC v3.15) + test
- remove the obsolete 'pathlib' backport dependency: it shadows the stdlib and
breaks tools that import pathlib on Python 3.10+ (broke YAC's Cython build)
* Reference #1561 in numba version pin comment
---------
Co-authored-by: Christopher Dupuis <45972964+cmdupuis3@users.noreply.github.com>
* Bump actions/download-artifact in the actions group across 1 directory (#1572)
Bumps the actions group with 1 update in the / directory: [actions/download-artifact](https://github.com/actions/download-artifact).
Updates `actions/download-artifact` from 7 to 8
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v7...v8)
---
updated-dependencies:
- dependency-name: actions/download-artifact
dependency-version: '8'
dependency-type: direct:production
update-type: version-update:semver-major
dependency-group: actions
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* [pre-commit.ci] pre-commit autoupdate (#1565)
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.15.20 → v0.15.21](astral-sh/ruff-pre-commit@v0.15.20...v0.15.21)
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Accusphere: gca_gca benchmarks and better arc sampling
* Accusphere: fix sum_of_squares accuracy bug + tests
* Accusphere: numba fma refactor and fix validation tautology
* Accusphere: scalarized _counts_as_crossing
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: vakudo <127726617+vakudo@users.noreply.github.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Rajeev Jain <rajeeja@gmail.com>
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
Co-authored-by: zarzycki <colin.zarzycki@gmail.com>
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Add thread-scaling benchmark for FP64 vs AccuX constlat dispatcher
* Cap thread-scaling sweep at performance cores to avoid E-core artifact
* Fix thread-scaling plot ticks to show only 1,2,4,8
* o Fix pre-commit
* o doc fixes
* o Remove benchmark image from repo
* Split out point-in-face and lat-lon bounds to a separate PR
* o computing: match AccuSphGeom acc_sqrt_re exactly (residual order, branch-free)
* o intersections: compensated norm in AccuXGCA kernel, branch-free mask validity per AccuSphGeom
* o arcs: branch-free on_minor_arc int mask at tol 1e-8 matching AccuSphGeom
* o benchmarks: drop point-in-polygon kernel bench, moved to point-in-face PR
* o benchmarks: drop thread-scaling constlat, moved to separate scaling issue
* o docs: move spherical-geometry notebook out, depends on split-out point-in-face
* o docs: drop spherical-geometry notebook from user-guide toctree
* o test_plot: drop to_raster_auto_extent, unrelated to AccuXGCA/AccuXConstLat scope
* o fix on_minor_arc antipodal false positive
* o add on_minor_arc degeneracy regression tests
* fix diff_of_products addition grouping to match AccuSphGeom eft.hpp
* o intersections: collapse pos/neg-valid branches via XOR in gca_const_lat_intersection
* Parallel partial edge_nodes
* Lazy intersections
* zonal and cross-section peakmem benchmarks
* forgot to save; fixes unmerged code
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* _flatnonzero missleading docstring
* Regression tests for crash and chunked grid crash
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Rajeev Jain <rajeeja@gmail.com>
Co-authored-by: Hongyu Chen <hyvchen@ucdavis.edu>
Co-authored-by: Rajeev Jain <1466114+rajeeja@users.noreply.github.com>
Co-authored-by: vakudo <127726617+vakudo@users.noreply.github.com>
Co-authored-by: Sam Evans <47793072+Sevans711@users.noreply.github.com>
Co-authored-by: Orhan Eroglu <32553057+erogluorhan@users.noreply.github.com>
Co-authored-by: zarzycki <colin.zarzycki@gmail.com>
Co-authored-by: Sam Evans <s7evans11@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rajeejarajeejarajeeja approved these changes
@erogluorhanerogluorhanAwaiting requested review from erogluorhan

Assignees

@Sevans711Sevans711

Labels

bugSomething isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zarzycki@Sevans711@rajeeja