Skip to content

test: add property-based tests for ak.array_equal#3891

Merged
ikrommyd merged 5 commits into
scikit-hep:mainfrom
TaiSakuma:TaiSakuma/test-array-equal-properties
Mar 24, 2026
Merged

test: add property-based tests for ak.array_equal#3891
ikrommyd merged 5 commits into
scikit-hep:mainfrom
TaiSakuma:TaiSakuma/test-array-equal-properties

Conversation

@TaiSakuma

@TaiSakuma TaiSakuma commented Feb 20, 2026

Copy link
Copy Markdown
Member

Following #3887, this PR adds another property-based test.

  • The PR adds property-based test for ak.array_equal().
  • This test is more fundamental than those for ak.to_buffers() and ak.from_buffers(), added in test: introduce property-based testing with to_buffers/from_buffers roundtrip #3887
  • I should've added this test first.
  • The test fails currently.
  • It will be easier to write other tests when this test passes.
  • The code is generated by Claude Code.
  • I'm writing this PR description.
  • Claude Code will open a PR with the GitHub CLI.

…symmetry)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov

codecov Bot commented Feb 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.08%. Comparing base (95fa2ff) to head (6a78845).

Additional details and impacted files

see 58 files with indirect coverage changes

@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/PR3891

@TaiSakuma
TaiSakuma requested review from ianna and ikrommyd February 20, 2026 16:35
@ikrommyd

Copy link
Copy Markdown
Member

This will need to wait for the fixes that I’m doing.

@ikrommyd

Copy link
Copy Markdown
Member

@TaiSakuma the error in CI here seems to be the one I mentioned in #3909 (comment). Could you take a look? It looks like it's raised inside hypothesis.

@TaiSakuma

TaiSakuma commented Mar 13, 2026

Copy link
Copy Markdown
Member Author

@ikrommyd The phases option should help.

from hypothesis import Phase

@settings(max_examples=200, phases=(Phase.generate,))

Hypothesis failed to find the simplest sample causing the error likely because the data structure is too complex.
With the above phases option, Hypothesis won't search for the simplest sample and will just print the first sample that fails the test.

@ikrommyd

Copy link
Copy Markdown
Member

I got this very quickly #3921. We'll see if any other issues arise here after that.

@ikrommyd

Copy link
Copy Markdown
Member

CI is passing now after my fix.
@maxymnaumchyk know anything about the GPU failures? They are not related to the PR but this is a new one for me:


>   ???
E   RuntimeError: Failed building segmented_sort, error code: 999
E   
E   This error occurred while calling
E   
E       ak.sort(
E           <Array [[[7, 2, 3], [4, 5, 6]]] type='1 * var * var * int64'>
E           axis = -1
E       )


cuda/compute/_bindings_impl.pyx:2541: RuntimeError

@maxymnaumchyk

Copy link
Copy Markdown
Collaborator

I started seeing the same error in my PR too. I will ask Ashwin.

@maxymnaumchyk

Copy link
Copy Markdown
Collaborator

I also see that building an environment is failing at Run GPU Tests (13). Maybe there is some versions incompatibility?

@ikrommyd

Copy link
Copy Markdown
Member

Okay yeah. I was just letting you know. They are not required to merge things so as long as a PR does not touch GPU stuff, we are fine.

@ikrommyd

Copy link
Copy Markdown
Member

@TaiSakuma is this good to go from your side?

@TaiSakuma

Copy link
Copy Markdown
Member Author

@ikrommyd Yes. It is great that these tests pass now.

@ikrommyd
ikrommyd marked this pull request as ready for review March 19, 2026 20:33
@maxymnaumchyk

Copy link
Copy Markdown
Collaborator

From what it seems, there is a version incompatibility. So, if we pin cuda-version==12.9 here, it would fix the RuntimeError: Failed building segmented_sort, error code: 999. Although I don't know if that is the right thing, as we specifically are testing for older cuda-version: - 12 there.

@ianna

ianna commented Mar 24, 2026

Copy link
Copy Markdown
Member

From what it seems, there is a version incompatibility. So, if we pin cuda-version==12.9 here, it would fix the RuntimeError: Failed building segmented_sort, error code: 999. Although I don't know if that is the right thing, as we specifically are testing for older cuda-version: - 12 there.

From CUDA installation instructions:

https://docs.nvidia.com/cuda/cuda-installation-guide-linux/

The following metapackages will install the latest version of the named component on Linux for the indicated CUDA version. “cu12” should be read as “cuda12”.

nvidia-cublas-cu12
nvidia-cuda-cccl-cu12
nvidia-cuda-cupti-cu12
nvidia-cuda-nvcc-cu12
nvidia-cuda-nvrtc-cu12
nvidia-cuda-opencl-cu12
nvidia-cuda-runtime-cu12
nvidia-cuda-sanitizer-api-cu12
nvidia-cufft-cu12
nvidia-curand-cu12
nvidia-cusolver-cu12
nvidia-cusparse-cu12
nvidia-npp-cu12
nvidia-nvfatbin-cu12
nvidia-nvjitlink-cu12
nvidia-nvjpeg-cu12
nvidia-nvml-dev-cu12
nvidia-nvtx-cu12

These metapackages install the following packages:

nvidia-cublas-cu129
nvidia-cuda-cccl-cu129
nvidia-cuda-cupti-cu129
nvidia-cuda-nvcc-cu129
nvidia-cuda-nvrtc-cu129
nvidia-cuda-opencl-cu129
nvidia-cuda-runtime-cu129
nvidia-cuda-sanitizer-api-cu129
nvidia-cufft-cu129
nvidia-curand-cu129
nvidia-cusolver-cu129
nvidia-cusparse-cu129
nvidia-npp-cu129
nvidia-nvfatbin-cu129
nvidia-nvjitlink-cu129
nvidia-nvjpeg-cu129
nvidia-nvml-dev-cu129
nvidia-nvtx-cu129

@ikrommyd

Copy link
Copy Markdown
Member

We can merge this one anyways though since GPU tests are not required and the tests added here have nothing to do with GPUs.

@ianna ianna left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@TaiSakuma - thanks, this is great! I don’t think we need to wait for GPU runners to be fixed before we merge this one. Thanks

@ianna

ianna commented Mar 24, 2026

Copy link
Copy Markdown
Member

We can merge this one anyways though since GPU tests are not required and the tests added here have nothing to do with GPUs.

Sure, go ahead👍

@ikrommyd
ikrommyd merged commit a2c2552 into scikit-hep:main Mar 24, 2026
37 of 39 checks passed
@TaiSakuma
TaiSakuma deleted the TaiSakuma/test-array-equal-properties branch April 22, 2026 20:45
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.

4 participants