Skip to content

feat: add argmin implementation using CCCL#3811

Merged
ianna merged 3 commits intoscikit-hep:mainfrom
maxymnaumchyk:maxymnaumchyk/cccl-argmin
Jan 21, 2026
Merged

feat: add argmin implementation using CCCL#3811
ianna merged 3 commits intoscikit-hep:mainfrom
maxymnaumchyk:maxymnaumchyk/cccl-argmin

Conversation

@maxymnaumchyk
Copy link
Copy Markdown
Collaborator

No description provided.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.49%. Comparing base (1b7e3d6) to head (b1b6f0f).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/awkward/_connect/cuda/_compute.py 0.00% 20 Missing ⚠️
src/awkward/_backends/cupy.py 0.00% 2 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/awkward/_connect/cuda/__init__.py 0.00% <ø> (ø)
src/awkward/_backends/cupy.py 43.47% <0.00%> (-1.98%) ⬇️
src/awkward/_connect/cuda/_compute.py 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maxymnaumchyk
Copy link
Copy Markdown
Collaborator Author

Interestingly, test_3459_virtualarray_with_cuda.py::test_recordarray_argmin_y_field passes, but ::test_recordarray_argmin_x_field does not.

@github-actions
Copy link
Copy Markdown

The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR3811

@maxymnaumchyk maxymnaumchyk marked this pull request as ready for review January 21, 2026 11:07
Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxymnaumchyk - Great! Thanks for implementing it. There was an ak.prod failure on the complex numbers in a block boundary test. It's unrelated to this PR and it was gone after re-running the tests - so I'm merging it. Thanks.

@ianna ianna merged commit 0f7ecc8 into scikit-hep:main Jan 21, 2026
60 of 61 checks passed
@ikrommyd
Copy link
Copy Markdown
Collaborator

Interestingly, test_3459_virtualarray_with_cuda.py::test_recordarray_argmin_y_field passes, but ::test_recordarray_argmin_x_field does not.

The difference is that the field x is a listoffsetarray and y a numpyarray. Virtual arrays rarely have anything to do with it as the buffers just get loaded before any kernels run.
In CI i also see this

This error occurred while calling

    ak.argmin(
        <Array [[1.1, 2.2], [3.3, 4.4], ..., []] type='5 * var * float64'>
        axis = 1
    )

which makes me think that probably the error happened on a fully materialized jagged array.
I'd suggest investigating this further (without any virtual arrays at all first) because it could be a real bug somewhere.

@maxymnaumchyk
Copy link
Copy Markdown
Collaborator Author

@ikrommyd you mean to test test_recordarray_argmin_x_field() just with RecordArrays? Like this> test_recordarray_argmin_x_field(recordarray, recordarray)

If so, then everything works on my machine~

@ikrommyd
Copy link
Copy Markdown
Collaborator

@ikrommyd you mean to test test_recordarray_argmin_x_field() just with RecordArrays? Like this> test_recordarray_argmin_x_field(recordarray, recordarray)

If so, then everything works on my machine~

I would personally just extract the test in a small snippet and not use any fixtures and pytest. I'd first try with a fully materialized array and then with virtual arrays.

@ianna
Copy link
Copy Markdown
Member

ianna commented Jan 21, 2026

@ikrommyd you mean to test test_recordarray_argmin_x_field() just with RecordArrays? Like this> test_recordarray_argmin_x_field(recordarray, recordarray)
If so, then everything works on my machine~

I would personally just extract the test in a small snippet and not use any fixtures and pytest. I'd first try with a fully materialized array and then with virtual arrays.

Thanks, @ikrommyd ! Your help is greatly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants