You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
seen on astropy's test suite, failures started a bit over 24h ago, which I believe corresponds to a nightly release being dropped.
I could reproduce this crash seemingly deterministically on a linux machine (but couldn't reprod on macos), and bisected the crash to 0f79a3a (which is the merge commit for gh-29270)
(I'm opening from a different computer than the one I can reproduce from, I'll complete the report as an edit)
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7ca9adf in unlink_chunk (p=p@entry=0x39e9ee0, av=0x7ffff7e03ac0 <main_arena>) at ./malloc/malloc.c:1610
warning: 1610 ./malloc/malloc.c: No such file or directory
I'll have a look, valgrind is a bit more useful (as this seems to lead to very cryptic failures later otherwise), but this fails assigning a subarray dtype with the fast path, so let's see.
However, it is doing so even though the array is not large enough for this! Now, NumPy wants to copy the whole itemsize (without looking at how big the thing actually is).
So, astropy's shape + stride + dtype.itemsize goes beyond the allocation boundaries here and that breaks the fast-path, the fast path itself is doing the right thing.
Now... I can disable the fast-path for this case. A bit annoying, because it does make sense for manually padded dtypes, but we can at least defer activating it for a while (depending on whether you can fix it or not maybe.)
The thing that makes this a bit non-obvious is is that the Distribution seems to enforce this dtype.itemsize == stride thing, even though it should be dtype.itemsize + offset == stride (where offset is the offset into the original dtype that this sub-field has... or identically the offset into the original data if we know it's a view with an offset).
I guess @mhvk might also be someone to ping (once again)... As I said: The hot-fix will be to not allow the fast-path for dtypes created with manual itemsize=..., which I would like to use more (related to GPU where larger alignment needs are more common).
Oops, that's a bit of a mess. As you can see from the substantial comments around the relevant astropy part, this piece required more trickery than I would have liked, and I don't think it is easy to change (since itemsize has to match a particular value). But it would be good to keep it working, and elsewhere numpy is very careful to only care about what data is actually being accessed, so possibly the same should happen here (or the fast path should check and just give up if the copy could go wrong; not sure if that's easy to implement...).
I think the only easy thing is to disable the fast path here. We could allow passing it as a new kwarg so users can set it one way or the other and then you can pass itemsize=..., trivial_copy=True (although it'll have to fail if something nested isn't a trivial copy), maybe.
Hello. astropy will ship RC soon and having a clean CI (even with numpy-dev) would be desirable. Is there any update on this issue? If not, any stop-gap we can do downstream in astropy itself? Thank you!
I opened a PR, I might like to add an option to the dict to enable the optimization again, but if you want to unblock CI quickly, let's just put that in (that doesn't need to be in 2.5 I think, either). (Busy long weekend here, so may not follow up until monday.)
If I there add offsets=[0], things pass even before #31371. The reason is that in that case, one forces is_dtype_struct_simple_unaligned_layout to be run; see
Activity
seberg commentedon Apr 30, 2026
Arg :(. sorry about that, I'll have a look... good chance that we can just run valgrind or so to dig into it.
neutrinoceros commentedon Apr 30, 2026
It took me longer than I care to admit to setup my env so that I could run with
spin gdb, and the output is a bit disappointing.seberg commentedon Apr 30, 2026
I'll have a look, valgrind is a bit more useful (as this seems to lead to very cryptic failures later otherwise), but this fails assigning a subarray dtype with the fast path, so let's see.
seberg commentedon Apr 30, 2026
TL;DR: Maybe we can fix astropy but defer this optimization for the
itemsize=usage for a release or two?Tricky, because it breaks the improvement there :(. There are two itemsizes involved here:
Astropy is intentionally padding a dtype here: https://github.com/astropy/astropy/blob/15891930c2fefa611d4678c9d080a4ef6eb2e83e/astropy/uncertainty/core.py#L76
However, it is doing so even though the array is not large enough for this! Now, NumPy wants to copy the whole itemsize (without looking at how big the thing actually is).
So, astropy's
shape + stride + dtype.itemsizegoes beyond the allocation boundaries here and that breaks the fast-path, the fast path itself is doing the right thing.Now... I can disable the fast-path for this case. A bit annoying, because it does make sense for manually padded dtypes, but we can at least defer activating it for a while (depending on whether you can fix it or not maybe.)
The thing that makes this a bit non-obvious is is that the Distribution seems to enforce this
dtype.itemsize == stridething, even though it should bedtype.itemsize + offset == stride(whereoffsetis the offset into the original dtype that this sub-field has... or identically the offset into the original data if we know it's a view with an offset).I guess @mhvk might also be someone to ping (once again)... As I said: The hot-fix will be to not allow the fast-path for dtypes created with manual
itemsize=..., which I would like to use more (related to GPU where larger alignment needs are more common).mhvk commentedon Apr 30, 2026
Oops, that's a bit of a mess. As you can see from the substantial comments around the relevant astropy part, this piece required more trickery than I would have liked, and I don't think it is easy to change (since
itemsizehas to match a particular value). But it would be good to keep it working, and elsewhere numpy is very careful to only care about what data is actually being accessed, so possibly the same should happen here (or the fast path should check and just give up if the copy could go wrong; not sure if that's easy to implement...).pllim commentedon Apr 30, 2026
Thanks for digging at this, all! The code was added in astropy/astropy#15304
seberg commentedon Apr 30, 2026
I think the only easy thing is to disable the fast path here. We could allow passing it as a new kwarg so users can set it one way or the other and then you can pass
itemsize=..., trivial_copy=True(although it'll have to fail if something nested isn't a trivial copy), maybe.pllim commentedon May 1, 2026
Hello. astropy will ship RC soon and having a clean CI (even with numpy-dev) would be desirable. Is there any update on this issue? If not, any stop-gap we can do downstream in astropy itself? Thank you!
seberg commentedon May 1, 2026
I opened a PR, I might like to add an option to the dict to enable the optimization again, but if you want to unblock CI quickly, let's just put that in (that doesn't need to be in 2.5 I think, either). (Busy long weekend here, so may not follow up until monday.)
mhvk commentedon May 2, 2026
Just a note without much thought about implications. In astropy, we create a weird dtype in https://github.com/astropy/astropy/blob/80447a4334ba617c04d5918b66d80793f202c52e/astropy/uncertainty/core.py#L160-L179
If I there add
offsets=[0], things pass even before #31371. The reason is that in that case, one forcesis_dtype_struct_simple_unaligned_layoutto be run; seenumpy/numpy/_core/src/multiarray/descriptor.c
Lines 1334 to 1337 in 8d78a99
So, there is a fix on the astropy side even without having to add a new keyword argument.