Skip to content

Rework geoloc to support yaw steering and numba for optimisation#229

Open
mraspaud wants to merge 5 commits intopytroll:mainfrom
mraspaud:rework-geoloc
Open

Rework geoloc to support yaw steering and numba for optimisation#229
mraspaud wants to merge 5 commits intopytroll:mainfrom
mraspaud:rework-geoloc

Conversation

@mraspaud
Copy link
Copy Markdown
Member

@mraspaud mraspaud commented Apr 2, 2026

This PR reworks the geoloc module, by providing generic instrument definition builders.

Moreover, we implement yaw-steering and add (optional) numba optimisation.

For generating a full swath of lon/lat, the speed-up goes from 5x to 25x depending on the geometry.

I used AI for this work.

  • Tests added
  • Fully documented

@mraspaud mraspaud self-assigned this Apr 2, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 80.36891% with 149 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.69%. Comparing base (682f9aa) to head (71c6057).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
pyorbital/geoloc.py 57.98% 142 Missing ⚠️
pyorbital/geoloc_instrument_definitions.py 96.47% 5 Missing ⚠️
pyorbital/tests/test_geoloc.py 99.27% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
- Coverage   90.48%   88.69%   -1.80%     
==========================================
  Files          19       19              
  Lines        3048     3679     +631     
==========================================
+ Hits         2758     3263     +505     
- Misses        290      416     +126     
Flag Coverage Δ
unittests 88.69% <80.36%> (-1.80%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread pyorbital/geoloc.py
Comment on lines +27 to +29
# Module-level cached Transformer — avoids re-creating the PROJ context on
# every get_lonlatalt() call.
_GEOCENT_TO_LATLONG = None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be simpler to do an @cache decorator? Essentially the same thing. Or is there really a measurable performance difference in just recreating it?

Comment thread pyorbital/geoloc.py
global _GEOCENT_TO_LATLONG
if _GEOCENT_TO_LATLONG is None:
_GEOCENT_TO_LATLONG = Transformer.from_crs(
dict(proj="geocent"), dict(proj="latlong"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are there EPSG or pyproj constant versions of these? It seems odd to create these CRSes from the PROJ.4-style dictionary/string rather than something more explicit. Especially if a and b radii can be specified or use an existing definition that match the A and B global in this module.

Comment thread pyorbital/geoloc.py
Comment on lines +549 to +552
if _HAS_NUMBA:
lon, lat, alt = _numba_ecef_to_lonlatalt(pos[0], pos[1], pos[2])
else:
lon, lat, alt = _get_transformer().transform(*(pos * 1000))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So all of this is to say that the numba version is faster than pyproj? Is that because of the parallel computing part of it or something else in the way the algorithm is implemented?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is no magic in the implemented algorithm, so just the parallelism + jit made it much faster than pyproj

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well pyproj and PROJ are both compiled so I'd assume it is purely the parallelism. I'm mostly worried about numba as a dependency when depending on Cython and openmp (using prange) may be "easier" in terms of getting this optimization all the time or at least in more instances/environments since openmp is also often required for pykdtree optimizations. There would be no conditionals of which function is used, it would just be a C version. Or...dask could be an option too since it is still parallel but with an overhead. Obviously you've already implemented this solution so it is hard to not use it and implement something else.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants