fix: cast y to float64 for numba crand in Moran_Local and G_Local to avoid TypingError.#427
fix: cast y to float64 for numba crand in Moran_Local and G_Local to avoid TypingError.#427samay2504 wants to merge 2 commits intopysal:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a numba TypingError triggered when float32 input arrays are passed into conditional randomization paths for local statistics by ensuring inputs are cast to float64 early.
Changes:
- Cast
ytonp.float64at the start ofMoran_Local. - Cast
x/ytonp.float64at the start ofMoran_Local_BV. - Cast
ytonp.float64at the start ofG_Localto stabilize thecrand/numba dot-product dtype behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
esda/moran.py |
Forces Moran_Local and Moran_Local_BV inputs to float64 to prevent dtype mismatches in numba-compiled conditional randomization. |
esda/getisord.py |
Forces G_Local inputs to float64 to prevent dtype mismatches in numba-compiled conditional randomization. |
Comments suppressed due to low confidence (2)
esda/moran.py:1783
- Avoid using
assertfor runtime input validation here (asserts are stripped withpython -O). Please raise aValueError(orTypeError) whenxandylengths differ so this check is enforced in production.
self.y = y
self.x = x
n = len(y)
assert len(y) == len(x), "x and y must have the same shape!"
self.n = n
esda/moran.py:1309
- Regression coverage appears to be missing for the float32/Numba conditional-randomization path. Please add a test that
Moran_Localwithy.astype(np.float32)andpermutations>0runs without raising anumba.core.errors.TypingError(skip if numba unavailable) and produces finitep_sim/simoutputs.
y = np.asarray(y, dtype=np.float64).flatten()
self.y = y
n = len(y)
self.n = n
self.n_1 = n - 1
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| x = np.asarray(x, dtype=np.float64).flatten() | ||
| y = np.asarray(y, dtype=np.float64).flatten() | ||
| self.y = y |
| assert transform.lower() in ("r", "b"), ( | ||
| f'Transforms must be binary "b" or row-standardized "r".Recieved: {transform}' | ||
| ) |
| def _infer_star_and_structure_w(weights, star, transform): | ||
| assert transform.lower() in ("r", "b"), ( | ||
| f'Transforms must be binary "b" or row-standardized "r".Recieved: {transform}' |
| y = np.asarray(y, dtype=np.float64).flatten() | ||
| self.n = len(y) | ||
| self.y = y | ||
| w, star = _infer_star_and_structure_w(w, star, transform) | ||
| if isinstance(w, W): |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #427 +/- ##
=======================================
- Coverage 82.7% 82.3% -0.4%
=======================================
Files 27 27
Lines 3833 3993 +160
=======================================
+ Hits 3170 3288 +118
- Misses 663 705 +42
🚀 New features to boost your workflow:
|
|
Fix the diff please. This doesn't require a change of 728 lines. |
…d replace assert with ValueError in getisord.py
516bdf9 to
e21c8ab
Compare
|
Thanks for the feedback @martinfleis , the issue has been addressed. |
|
Hi! thank you for this contribution. Casting all input to |
|
Thanks for the guidance @ljwolf , I’ve updated this to use explicit Numba eager signatures for both float32 and float64 paths instead of coercing inputs to float64. I removed the blanket casts in esda/moran.py and esda/getisord.py, added typed local crand signatures for univariate and bivariate kernels, and added targeted float32 regression coverage in esda/tests/test_moran.py and esda/tests/test_getisord.py. The focused float32 tests are passing on this branch. |
fixes #376.
This fixes issue where passing a float32 array to Moran_Local causes a numba TypingError. When you pass a float32 array to Moran_Local, Moran_Local_BV or G_Local the internal standardised array keeps that float32 type. The conditional randomisation engine then casts the spatial weights to match this type and that creates a nasty mismatch inside the numba compiled functions when they try to multiply float32 and float64 values together. I fixed this by casting the input arrays to float64 right at the start of these functions to make sure the internal arrays always have the right type. It completely gets rid of the numba error and lets conditional randomisation work perfectly for float32 inputs without changing the behaviour for standard float64 arrays. I also threw in some regression tests for all three methods so we don't see this problem creeping back in.