-
Notifications
You must be signed in to change notification settings - Fork 16
Defer xgcm import to speed up xcdat startup time by ~3 seconds
#810
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
Conversation
xgcm to .vertical() methodxgcm in .vertical()
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #810 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1784 1784
=========================================
Hits 1784 1784 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements lazy importing of xgcm.Grid to improve module import performance by avoiding unnecessary JIT compilation overhead during initial imports.
Key changes:
- Moved
xgcm.Gridimport from module-level to inside theverticalmethod (lazy import) - Updated all test mocks to patch
xgcm.Gridinstead ofxcdat.regridder.xgcm.Grid - Added documentation explaining the rationale for lazy importing
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| xcdat/regridder/xgcm.py | Removed module-level Grid import and added lazy import with explanatory comment in the vertical method |
| tests/test_regrid.py | Updated 8 mock.patch decorators to patch xgcm.Grid instead of the old module-level import path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tomvothecoder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jasonb5 and @pochedls, I've opened this PR to address #805. Can you review to ensure it looks good? It's a simple one line change.
This PR defers the import of xgcm to only when .vertical() is called to improve the speed of import xcdat. Please refer to the PR description for more information.
Thanks!
xgcm in .vertical() xgcm import to speed up xcdat startup time by ~3 seconds
jasonb5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR significantly improves the import time of
xcdatby deferring the import of thexgcmpackage until it is actually needed within the.vertical()accessor method.Problem
The
xgcm.transformmodule uses Numba's@guvectorize, which eagerly compiles functions at import time. Sincexcdat/__init__.pyindirectly importsxgcm, this causes an additional 3–4 seconds of import latency, even if vertical regridding is never used.More context here: #805 (comment)
Solution
This PR resolves the issue by:
from xgcm import Gridstatement until the moment.vertical()is called.xgcmand its Numba-based modules are only loaded and compiled if/when vertical regridding is actually used.Results
Import-time benchmarking shows a ~3 second improvement in
xcdatimport latency:mainxgcmfeature/805-speed-up-importsxgcmimportReproducible Benchmark Script
The script is included to measure true import-only performance and verify the improvement.
Checklist
If applicable: