ENH: Add ufunc flag to request contiguous buffers for SIMD loops by Aniketsy · Pull Request #30449 · numpy/numpy

Aniketsy · 2025-12-16T17:08:02Z

Add NPY_UFUNC_REQUIRE_CONTIGUOUS flag and a flags field to PyUFuncObject.
Update code generation to set this flag for SIMD optimized ufuncs (sin, cos).
Change the iterator setup so it gives contiguous buffers to these ufuncs.
Simplify the SIMD loops for sin and cos to always expect contiguous data.
Add tests

Please let me know if my approach or fix needs any improvements . I’m open to feedback and happy to make changes based on suggestions.
Thank you !

… loops

seberg · 2025-12-17T11:32:29Z

Thanks for looking into this. But this is not the right abstraction layer to start with. We should handle this in the ArrayMethod instead. And yes, that will make it awkward to use it for existing sin/cos implementations, because setting the flag there may not be straight-forward (something like this here may still be needed/useful).

But, I think we need to start at the ArrayMethod integration. Also, as you can see just using the flag in the one iterator path is not sufficient, ufuncs have multiple paths and all of them need to ensure this.
(Including things like np.add.reduce, etc.)
This shouldn't be extremely complicated, but there is quite a bit more ground to cover.

Aniketsy · 2025-12-18T04:45:18Z

Thanks for the detailed explanation, I’ll go with your suggestion and look into handling this at the ArrayMethod integration. I’ll reach out here if I get stuck or need clarification.

Aniketsy · 2026-01-08T09:24:39Z

@seberg I did try to work towards moving this into the ArrayMethod integration, but I’m currently a bit stuck on how to correctly handle this across all ufunc execution paths.

Rather than risk pushing an incomplete or incorrect fix, I think it’s best for me to step back and let someone with more experience in this area take it forward.

Please let me know whether you’d prefer I close the PR, or leave it open in case it’s useful as a reference.

seberg · 2026-01-08T12:14:07Z

but I’m currently a bit stuck on how to correctly handle this across all ufunc execution paths.

Yeah, that may be quite tricky (or at least unwieldy, to make sure you get all).

If you like, maybe push your start here and comment on the issue. Then we can close this PR but it may help someone else to dive in.
(If you care, it may also mean that your start ends up in the history if it gets picked up, depending on how far it is.)

(I am not actually sure how easy it is, for some places it should be pretty straight forward, but unwieldy, for gufuncs we may want to just reject it for now also.)

mhvk · 2026-01-08T16:21:52Z

Just commenting that this would be great to have for other purposes too -- I guess it means the step of converting all the old loops to array methods is becoming more pressing...

Aniketsy · 2026-01-08T16:35:10Z

Thanks for the insight, that makes sense. I don’t have much experience with this part of the codebase yet, so I didn’t want to create extra work for reviewers. If the direction of these changes looks good, I’d be happy to explore this further and would appreciate any guidance on how best to approach it.

seberg · 2026-01-08T17:22:15Z

At a very brief glance the direction makes sense. One thing that probably is a good to simplify this is to not use it for the sin/cos. For example, we could make the add_sfloats loop use that (it's testing only for the _scaled_float_dtype.c tests).

Another note is that if that flag is set (or we can actually set it for the user), we should allow users to only give a contiguous_loop when creating the ufunc (you would see this when looking at the add_sfloats example).

We may not be able to cover quite all things with that one implementation, but most paths for sure. (E.g. I think casts behave the same way, but need a different logic, we may want to reject the flag for casts for now, just as it may be easier to reject it for gufuncs.)

Hard to guarantee that we'll progress quickly. But if you are happy to prod it along, we can maybe also heavy handedly contribute to this at some point.

Aniketsy · 2026-01-08T17:38:22Z

Thanks for the feedback. I’d be happy to work on this, and for now I’ll proceed with the points you mentioned.

Aniketsy · 2026-01-10T19:05:16Z

+    res = sfloat_add_loop("add",
+            bmeth->dtypes, (PyObject *)bmeth->method);
+    Py_DECREF(bmeth);
+    if (res < 0) {


I only made two actual changes here, the remaining changes come from resolving merge conflicts.

Aniketsy · 2026-04-06T08:52:40Z

hii @seberg could you please review this, when you get chance and give some pointers to proceed with this, as we have still ci failures, sorry for delay progress. Thank you 😊 .

seberg

Thanks, I think this looks pretty good, but the test needs to be modified to actually check that the contiguous iteration is guaranteed (and also for all types of calls).

There is a separate path for casts, we are not implementing it there yet, so the cast registration path should probably reject the flag for now.

seberg · 2026-04-07T08:21:11Z

+    try:
+        from numpy._core import _scaled_float_dtype
+    except ImportError:
+        pytest.skip("Could not import _scaled_float_dtype for testing")


This should always be importable, even globally so.

seberg · 2026-04-07T08:21:32Z

+    assert_array_equal(c, expected)
+
+def test_ufunc_requires_contiguous_flag_sfloat():
+    #GH#30413


Suggested change

#GH#30413

# gh-30413

Just habit/nit (also above)

seberg · 2026-04-07T08:27:07Z

    PyType_Slot add_slots[3] = {
        {NPY_METH_resolve_descriptors, &add_sfloats_resolve_descriptors},
-        {NPY_METH_strided_loop, &add_sfloats},
+        {NPY_METH_contiguous_loop, &add_sfloats},


To really test this, the add_sfloats loop needs to have a check that either the length is <= 1 or the stride is indeed contiguous.
Otherwise, the test exercises the code paths, but doesn't actually check that contiguous is guaranteed.

seberg · 2026-04-07T08:27:46Z

+
+    c = a.copy()
+    np.add(c, b, out=c)
+    assert_array_equal(c, expected)


We should have plenty of tests for this, I think you can just remove this.

seberg · 2026-04-07T08:32:11Z

+
+    res = np.add(a_non_contig, a_non_contig)
+    expected = (a_raw[::2] + a_raw[::2]).astype(dt)
+    assert_equal(res, expected)


We need to add tests for the other code paths (accumulate, reduce, etc.).

Reduce is also different, in that even with the flag the non-reduced argument isn't actually contiguous at all.
So either we need to clearly explain that in the docs, or just keep using strided_loop for now for that path but of course with the promise that the strides are contiguous except for the reduce case.
(Or maybe we could add a contiguous_reduce_loop, but one can always implement that with a custom get_loop also.)

seberg · 2026-04-07T08:33:17Z

+                "Invalid flags set for ArrayMethod: 0x%x. (method: %s)",
+                spec->flags, spec->name);
+        return -1;
+    }


Hmmm, yeah, I guess this means if we add new flags (that don't really matter if there are there), downstream would have to guard by runtime version if on an older system.
But maybe that is fine, a check seems good as such really.

Aniketsy · 2026-04-08T04:16:08Z

@seberg thanks for the review, I've applied changes as per your suggestion. As we have ci-failure, I've try to debug and fix,
Also one point:--
why we need to build everytime, for running tests locally, is this like did i missed any steps during setup of my environment or it is common. Thankyou 😊

seberg

Thanks for the updates, small comments only. I am leaning towards requiring a strided_loop for simplicity right now TBH, even if it is a bit awkward.
(It seems to me we could move towards allowing just a contiguous loop from there.)

About testing, I would suggest you use a spin test workflow for NumPy. Unless you are doing a full pip install things should only rebuild what is needed (unless you modify some deep header files).

seberg · 2026-04-08T07:31:39Z

+    assert(dimensions[0] <= 1 ||
+           (strides[0] == sizeof(double) &&
+            strides[1] == sizeof(double) &&
+            strides[2] == sizeof(double)));


Please make this raise an error rather than an assert, since that way we are guaranteed to hit in during testing. Not quite sure you can use npy_gil_error here, but you can use an equivalent pattern.
(Otherwise it needs to be build with debugging but also it'll just hit an abort rather than a nice Python error.)

seberg · 2026-04-08T07:39:24Z

+        /*
+         * Note: for reductions, only the result operand is guaranteed to be
+         * contiguous here; the reduction axis for the input operand may still
+         * be non-contiguous.


There is public documentation for NPY_METH_contiguous_loop which would need to mention this part.
Although, I am wondering if we shouldn't still force passing a NPY_METH_strided_loop even if the input would be guaranteed to be contiguous or a reduction.
(That also then allows distinguishing a probable reduce, from a non-reduce. By basically promising that the strided_loop must get a contiguous stride or a stride of zero.)

seberg · 2026-04-09T07:05:21Z


 def test_ufunc_requires_contiguous_flag_sfloat():
    # gh-30413
+    _scaled_float_dtype = pytest.importorskip("numpy._core._scaled_float_dtype")


Fly-by: But we need to see why this import is failing, this is compiled with NumPy so it must be available. Probably the code is a bit wrong so you don't actually see the original error or so (I am not sure).

thanks, i'll look more into this and let you know if i get something .

@seberg need your help :) i haven't got much maybe if you have any feeling for any section where we could have done something wrong, I'd love dive there and debug . Thank you!

IIRC, it just failed very early on or for rather simple code with this? If you are up for it, I would try to run it through gdb/lldb as a first shot: https://numpy.org/devdocs/dev/development_advanced_debugging.html#building-with-debug-symbols

Scrolling through, I can't see a very obvious issue, so I would have to run it through gdb as well.

thanks , i'll also try to run and debug

seberg · 2026-04-13T08:07:06Z

+        if (context.method->flags & NPY_METH_REQUIRES_CONTIGUOUS) {
+            op_flags[0] |= NPY_ITER_CONTIG;
+            op_flags[1] |= NPY_ITER_CONTIG;
+            op_flags[2] |= NPY_ITER_CONTIG;


I wonder if this might make things break, since some reductions must have a stride of 0 and can't be contig, but dunno

Aniketsy · 2026-07-03T16:07:59Z

😿 we didn't had progress in this, but i have given a try in this before

thanks , i'll also try to run and debug

but don't remember where i got stuck... I'll start again in this and make sure this time it moves forward and hopefully get merged soon .

Aniketsy · 2026-07-07T12:28:12Z

@seberg gentle ping , could you please have a look in this, when you get chance. I've updated with some changes.

Aniketsy · 2026-07-21T20:16:01Z

hii @seberg just pinning you, in case you missed notification, sorry i didn't mean to annoy by repeated mentioning you as i understand , due to large no. of PRs it takes time to review ... please ping you when you get chance 😊

seberg · 2026-08-03T13:15:09Z

Sorry, I'll have to come back to it in a while unfortunately.

One reason for not jumping it, is that I think the contig flag to the iterator is dubious, i.e. see this commit and maybe especially the last test. So figuring out whether this will actually do the right thing is trickier then one would expect :/!

Anther note: The contiguity request flag could maybe be a runtime flag, either way, by adding it to SFloat, you are completely disabling the other tests that uses the strided versions.

Allow ufuncs to request contiguous buffers from the iterator for SIMD…

d6aa1fc

… loops

github-actions Bot added the 01 - Enhancement label Dec 16, 2025

Aniketsy marked this pull request as draft December 16, 2025 17:18

Allow ufuncs to request contiguous buffers from the iterator for SIMD

0a2b8cf

Aniketsy force-pushed the fix-30413 branch 2 times, most recently from 38af602 to 0a2b8cf Compare January 8, 2026 16:12

Allow ufuncs to request contiguous buffers from the iterator for SIMD

17f4af7

Allow ufuncs to request contiguous buffers from the iterator for SIMD

9f65e52

Aniketsy force-pushed the fix-30413 branch from cdbff79 to 9f65e52 Compare January 10, 2026 18:56

Aniketsy commented Jan 10, 2026

View reviewed changes

Aniketsy force-pushed the fix-30413 branch from a37bff8 to 9f65e52 Compare April 6, 2026 08:21

Merge branch 'main' into fix-30413

df47989

Allow ufuncs to request contiguous buffers from the iterator for SIMD

fa53ab6

Aniketsy marked this pull request as ready for review April 7, 2026 07:03

seberg reviewed Apr 7, 2026

View reviewed changes

Allow ufuncs to request contiguous buffers from the iterator for SIMD

c138e7d

seberg reviewed Apr 8, 2026

View reviewed changes

Aniketsy added 2 commits April 9, 2026 12:03

Allow ufuncs to request contiguous buffers from the iterator for SIMD

e268156

fix-lint

b81fb87

seberg reviewed Apr 9, 2026

View reviewed changes

seberg reviewed Apr 13, 2026

View reviewed changes

ENH: Add NPY_METH_REQUIRES_CONTIGUOUS flag for ufunc array methods

d6f3e08

Improve formatting in doc to fix ci

d968523

This comment was marked as off-topic.

Sign in to view

Conversation

Aniketsy commented Dec 16, 2025

Uh oh!

seberg commented Dec 17, 2025

Uh oh!

Aniketsy commented Dec 18, 2025

Uh oh!

Aniketsy commented Jan 8, 2026

Uh oh!

seberg commented Jan 8, 2026

Uh oh!

mhvk commented Jan 8, 2026

Uh oh!

Aniketsy commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seberg commented Jan 8, 2026

Uh oh!

Aniketsy commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aniketsy commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aniketsy commented Apr 8, 2026

Uh oh!

seberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aniketsy commented Jul 3, 2026

Uh oh!

Aniketsy commented Jul 7, 2026

Uh oh!

Aniketsy commented Jul 21, 2026

Uh oh!

seberg commented Aug 3, 2026

Uh oh!

This comment was marked as off-topic.

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Aniketsy commented Jan 8, 2026 •

edited

Loading

Aniketsy commented Apr 6, 2026 •

edited

Loading