Repository navigation
Give lazy expressions NumPy's result dtype for division and float functions - #754
FabianIsensee wants to merge 5 commits into
Conversation
…ctions check_dtype() gave integer or bool result dtypes to hypot, copysign and nextafter of integer subexpressions, to true division involving bools ((a < b) / b, bool / bool) and to the bool //, % and ** that NumPy computes in int8, so their results were truncated on every evaluation path: hypot(a + 1, b) of int8 arrays gave [2 3 4 5] for NumPy's [2.83 3.61 4.47 5.39], (a < b) / b gave [0 0 0 0] for [0.5 0 0 0]. It now asks NumPy for these operators and functions: the operator or ufunc on empty operands, with a scalar next to an array kept as it is, so that it promotes as a Python scalar; int32 / int32 keeps its float32. Array operands' dtypes are read directly instead of through blosc2.result_type(), which more than pays for the probe: building (a + b) * 2 - f takes 62 instead of 70 us. The new test checks the dtype on every path and the values where the typing follows NumPy (compute() and partial slices). expr[:] and reductions of integer true division and two-argument float functions of two integer arrays still compute in integer arithmetic inside the bundled miniexpr, which is for miniexpr to fix. Taken from Blosc#744 (55660b4), without its miniexpr guard. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thanks for this fix. I reviewed PR head Non-native-endian operands now fail existing dtype checksimport blosc2
import numpy as np
a = blosc2.asarray(np.arange(1, 7, dtype=np.int32))
b = np.ones(6, dtype=">i4")
print((a & b)[:])On the base, this returns The previous The same distinction also bypasses the intentional int32-division special case: for This concerns Validation: using the Posted by an AI coding assistant at the maintainer’s request. |
…probe _operand_dtype() returned an array operand's dtype as it is, but blosc2.result_type(), which it replaced, gave the native byte order, and check_dtype() compares against native dtypes. A big-endian operand therefore failed the bitwise operators (a & b with a >i4 NumPy array raised "Invalid operand type for &: >i4") and lost the float32 of int32 / int32 (a / a of a >i4 array gave float64). _operand_dtype() now returns np.result_type(dtype), which is what blosc2.result_type() computed, without building the array. Structured and void dtypes go through blosc2.result_type() as before, which rejects some of them. The probe also let arctan2 of complex, datetime or string operands build and fail only on evaluation; arctan2 raises NumPy's error when the expression is built again. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thanks, Francesc, you are right: Looking for more of the same, I found two other changes against the base and removed them:
To check for others, I compared the base (7db7b09) and this PR on about 116k expressions: all operators and two-argument functions, and the unary functions, on bool, all int, uint, float and complex dtypes in both byte orders, plus datetime, timedelta, string, void and structured; NDArray, NumPy, LazyExpr and scalar operands; built from functions and from strings; checking the dtype,
The comparison also turned up some problems that are on main already, unrelated to this PR. I can open issues for them if you like:
AI disclosure: the fix, the tests and the comparison were done with Claude Code; I reviewed them. Best, |
On WebAssembly, compute() and slices also run the bundled miniexpr, which computes integer true division and two-argument float functions of integer arrays in integer arithmetic (0.0 for NumPy's 0.5), so the test checks only the dtypes there. A Python int is int32 on wasm32, so int32 / 2 gives the intended float32 of int32 / int32 there; skip it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The previous commit checked only the dtypes on WebAssembly and skipped int32 / 2 there, which also dropped the value checks that pass. Keep the test as it was and mark exactly the 32 cases that fail on WebAssembly as xfail(strict=True) instead: integer true division and hypot/nextafter of integer arrays, which the bundled miniexpr computes in integer arithmetic there on compute() and slices too, and int32 / 2, which gets the float32 of int32 / int32 because a Python int is int32 on wasm32. With strict, a fix shows up as XPASS. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
One more thing: the WASM job on my fork showed that the new 30 of them are miniexpr's: on WebAssembly, I did not want to weaken the test, since these should pass once miniexpr is fixed, so b62727a marks exactly these 32 cases as AI disclosure: this fix was also done with Claude Code; I reviewed it. |
|
Thanks for the follow-up fixes and WASM investigation. The strict xfails are useful tracking, but I suggest separating the two kinds of WASM failures before merging:
Only items 1, 2, and 5 require code/test changes in this PR. The numerical fixes can follow separately, with tracking issues in place before merging; there is no need to expand this PR into a complete native typing fix. Does that split sound reasonable? I inspected the successful fork WASM CI run and its 32 expected failures; I have not independently run the WASM build locally. Posted by an AI coding assistant at the maintainer’s request. |
… test Where a Python int is int32, as on wasm32, int32_array / 2 is int32 / int32, which gives float32 on purpose. The test now expects that, values included, instead of marking these two cases xfail. The other 30 WebAssembly xfails stay strict, each with its reason and miniexpr issue (Blosc/miniexpr#26, Blosc/miniexpr#27). Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thanks, Francesc, the split sounds right. Done:
Fixes for both miniexpr issues are prepared; I can send them as miniexpr PRs whenever you want them. AI disclosure: the test change and the issues were done with Claude Code; I reviewed them. |
|
Hi Fabian, thanks for the fixes and the careful regression coverage here. Since this PR was opened, the changes on
I checked the added tests from this PR against the current branch ( Could you please close this PR as superseded and adapt any still-useful parts into a follow-up against the new typed evaluator, rather than just resolving the merge conflicts? In particular, selected additional regression tests and the operand-dtype performance optimization may still be useful. Please preserve strong/zero-dimensional scalar promotion, avoid duplicating the new dtype-probing machinery, and reassess the old WASM xfails against current behavior instead of carrying them over unchanged. Any remaining legacy-evaluator fixes can be considered separately. Thanks again for the investigation and tests! Posted by an AI coding assistant at the maintainer’s request. |
|
Hey @FrancescAlted, |
|
Hi Fabian. Yeah, miniexpr/blosc2 integration has been a moving target for the past month or so, and I hope to settle down specs very soon. Your work is important to reach stability, so I think that sending feedback now (via PRs or issues) would be a good thing right now. Go ahead and see how it goes. |
|
A lot of my fixes touched me_compile while you seem to be moving away from that. Is me_compile still relevant or will it be superseded by the Menudet graph engine? |
|
Yes, you’ve understood the direction correctly. We’re moving ordinary expression evaluation toward the typed Menudet graph engine, rather than the legacy The goal is portable execution with explicitly defined, NumPy-style numerical semantics: intermediate operations retain their own types instead of inheriting the final output dtype. The same validated graphs can execute natively through interpretation or optional JIT, including standalone and WASM deployments.
The best architectural reference is Native numerical graph preparation. For the numerical contract, see NumPy arithmetic semantics and NumPy function semantics. This is still evolving and not a claim of complete NumPy compatibility (which could be difficcult to achieve in its entirety). Those documents rather describe the supported subset and remaining qualification boundaries. |
|
Thanks Francesc, that makes the direction clear. While checking my PRs against the new default, I found that the typed evaluator is much slower than the legacy one for several common expressions (times in ms):
Legacy evaluator: yesterday's What I found so far:
Reproducer (5.2M items, a few seconds)import os, time
import numpy as np
import blosc2
def best(f, n=5):
f()
ts = []
for _ in range(n):
t = time.perf_counter(); f(); ts.append(time.perf_counter() - t)
return min(ts) * 1e3
rng = np.random.default_rng(0)
shape = (20, 512, 512) # 5.2M items
kw = {"chunks": (10, 512, 512), "blocks": (1, 128, 512)}
f32 = blosc2.asarray(rng.normal(100, 30, size=shape).astype(np.float32), **kw)
i32a = blosc2.asarray(rng.integers(-1000, 1000, size=shape, dtype=np.int32), **kw)
i32b = blosc2.asarray(rng.integers(1, 1000, size=shape, dtype=np.int32), **kw)
i16 = blosc2.asarray(rng.integers(-1000, 1000, size=shape, dtype=np.int16), **kw)
u8 = blosc2.asarray(rng.integers(0, 255, size=shape, dtype=np.uint8), **kw)
p = blosc2.asarray(rng.random(shape) > 0.5, **kw)
q = blosc2.asarray(rng.random(shape) > 0.3, **kw)
cases = {
"i16 + u8": ("a + b", {"a": i16, "b": u8}),
"i32 a % b": ("a % b", {"a": i32a, "b": i32b}),
"bool p | q": ("p | q", {"p": p, "q": q}),
"i32 a * 3 + 1": ("a * 3 + 1", {"a": i32a}),
"where((a > 90) & (a < 110), a, 0)": ("where((a > 90) & (a < 110), a, 0)", {"a": f32}),
}
print("BLOSC2_EXPRESSION_EVALUATOR =", os.environ.get("BLOSC2_EXPRESSION_EVALUATOR", "(default)"))
for name, (expr, ops) in cases.items():
getitem = best(lambda: blosc2.lazyexpr(expr, ops)[:])
compute = best(lambda: blosc2.lazyexpr(expr, ops).compute())
print(f"{name:36s} [:] {getitem:7.1f} ms compute() {compute:7.1f} ms")On On the miniexpr PRs: following your answer, I will open the ones that fix crashes, wrong values or speed in the existing paths without changing their semantics: heap corruption in AI disclosure: the measurements and this comment were prepared with Claude Code; I reviewed them. |
|
@FabianIsensee Yes, that kind of feedback and possible enhancements would be great, and it is the kind of help needed at this time. Looking forward to your PRs/issues to miniexpr. |
|
Closing this as superseded, as you suggested. The useful parts are in #758:
AI disclosure: the checks and this comment were done with Claude Code; I reviewed them. 🤖 Generated with Claude Code |
The dtype tests of Blosc#754, on the typed evaluator: float functions and true division of integer and bool subexpressions, bool //, % and **, non-native byte order, and arctan2() rejecting complex operands. The values of expr[:] are checked too. The WebAssembly xfails are gone; the cases the legacy evaluator still gets wrong are strict xfails there. New strict xfail: comparing an int8 array with 300 raises OverflowError in the typed evaluator, while NumPy 2 compares (all True). Co-Authored-By: Claude Opus 5.5 <[email protected]>
Hi Francesc,
a third PR into #741, independent of #752 and #753: it is based on
improved-miniexpr(7db7b09) and merges cleanly with both, in any order. It is the Python-side part of the miniexpr guard of #744, without the guard.The bug:
check_dtype()gave integer or bool result dtypes tohypot,copysignandnextafterof integer subexpressions, to true division involving bools, and to the bool//,%and**that NumPy computes in int8. Their results were truncated on every evaluation path, e.g. for int8 arrays:The fix: for these operators and functions,
check_dtype()now asks NumPy: the operator or ufunc on empty operands, with a scalar next to an array kept as it is, so that it promotes as a Python scalar.int32 / int32keeps its float32. Array operands' dtypes are read directly instead of throughblosc2.result_type(), which more than pays for the probe: building(a + b) * 2 - ftakes 62 instead of 70 µs.Tests:
test_lazyexpr_result_dtype_matches_numpy(17 expressions x bool/int8/uint16/int32/int64, built from functions and from strings). It checks the dtype on every path, and the values oncompute()and on partial slices. 64 of its cases fail without this change.pytest tests: 10757 passed. The 2 failures are theb2viewhttp tests that need aiohttp, as onmain.Still wrong, but not fixable here:
expr[:]and the reductions of integer true division and of two-argument float functions of two integer arrays still compute in integer arithmetic inside the bundled miniexpr:(a < b) / bnow has the right dtype, but[:]still gives[0. 0. 0. 0.].compute()gets them right, because it goes through the native graph. That is for miniexpr to fix; I'd like to send PRs there once this branch is in, as offered above.AI disclosure: the code, the tests and the measurements were done with Claude Code; I reviewed them.
Best,
Fabian
🤖 Generated with Claude Code