Skip to content

Fix logic in test_segmented_reduce - #4198

Merged
leofang merged 3 commits into
NVIDIA:mainfrom
oleksandr-pavlyk:correct-segmented-reduce-tests
Mar 24, 2025
Merged

leofang merged 3 commits into
NVIDIA:mainfrom
oleksandr-pavlyk:correct-segmented-reduce-tests

Conversation

@oleksandr-pavlyk

Copy link
Copy Markdown
Contributor

Description

closes gh-4197

Fix logic in test_segmented_reduce, also test over different types of offsets

To resolve gh-4197, use cupy.cumsum to accumulate over random partition sizes to form correct offsets sequence.

Add assertions to verify that offsets is a non-decreasing sequence, and that its last element equals the size of the input array.

Perform the test for several plausible offset data types.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

… offsets

To resolve NVIDIAgh-4197, use `cupy.cumsum` to accumulate over random partition
sizes to form correct offsets sequence.

Add assertions to verify that `offsets` is a non-decreasing sequence, and
that its last element equals the size of the input array.

Perform the test for several plausible offset data types.
@oleksandr-pavlyk
oleksandr-pavlyk requested a review from a team as a code owner March 19, 2025 16:30
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Mar 19, 2025
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Mar 19, 2025
@github-actions

Copy link
Copy Markdown
Contributor
🟥 CI finished in 14m 04s: Pass: 0%/1 | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
  • 🟥 python: Pass: 0%/1 | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s

    🟥 cpu
      🟥 amd64              Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    🟥 ctk
      🟥 12.8               Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    🟥 cudacxx
      🟥 nvcc12.8           Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    🟥 cudacxx_family
      🟥 nvcc               Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    🟥 cxx
      🟥 GCC13              Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    🟥 cxx_family
      🟥 GCC                Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    🟥 gpu
      🟥 rtx2080            Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    🟥 jobs
      🟥 Test               Pass:   0%/1   | Total: 14m 04s | Avg: 14m 04s | Max: 14m 04s
    

👃 Inspect Changes

Modifications in project?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

Modifications in project or dependencies?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

🏃‍ Runner counts (total jobs: 1)

# Runner
1 linux-amd64-gpu-rtx2080-latest-1

@github-actions

Copy link
Copy Markdown
Contributor
🟩 CI finished in 1h 12m: Pass: 100%/1 | Total: 1h 12m | Avg: 1h 12m | Max: 1h 12m
  • 🟩 python: Pass: 100%/1 | Total: 1h 12m | Avg: 1h 12m | Max: 1h 12m

    🟩 cpu
      🟩 amd64              Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    🟩 ctk
      🟩 12.8               Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    🟩 cudacxx
      🟩 nvcc12.8           Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    🟩 cudacxx_family
      🟩 nvcc               Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    🟩 cxx
      🟩 GCC13              Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    🟩 cxx_family
      🟩 GCC                Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    🟩 gpu
      🟩 rtx2080            Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    🟩 jobs
      🟩 Test               Pass: 100%/1   | Total:  1h 12m | Avg:  1h 12m | Max:  1h 12m
    

👃 Inspect Changes

Modifications in project?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

Modifications in project or dependencies?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

🏃‍ Runner counts (total jobs: 1)

# Runner
1 linux-amd64-gpu-rtx2080-latest-1

@github-actions

Copy link
Copy Markdown
Contributor
🟩 CI finished in 1h 10m: Pass: 100%/1 | Total: 1h 10m | Avg: 1h 10m | Max: 1h 10m
  • 🟩 python: Pass: 100%/1 | Total: 1h 10m | Avg: 1h 10m | Max: 1h 10m

    🟩 cpu
      🟩 amd64              Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    🟩 ctk
      🟩 12.8               Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    🟩 cudacxx
      🟩 nvcc12.8           Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    🟩 cudacxx_family
      🟩 nvcc               Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    🟩 cxx
      🟩 GCC13              Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    🟩 cxx_family
      🟩 GCC                Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    🟩 gpu
      🟩 rtx2080            Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    🟩 jobs
      🟩 Test               Pass: 100%/1   | Total:  1h 10m | Avg:  1h 10m | Max:  1h 10m
    

👃 Inspect Changes

Modifications in project?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

Modifications in project or dependencies?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

🏃‍ Runner counts (total jobs: 1)

# Runner
1 linux-amd64-gpu-rtx2080-latest-1

1. Use `cupy.random` to draw random sample on GPU, rather than on CPU
   followed by a transfer
2. Use `cp.empty` to allocate output, rather than `cp.zeros`
n_segments = 2**4
h_offsets = np.zeros(n_segments + 1, dtype="int64")
h_offsets[1:] = rng.multinomial(sz, [1 / 16] * 16)
rng = cp.random

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.

FWIW CuPy also has the new random interface (no need to change).

@leofang
leofang enabled auto-merge (squash) March 24, 2025 21:56
@github-actions

Copy link
Copy Markdown
Contributor
🟩 CI finished in 1h 16m: Pass: 100%/1 | Total: 1h 16m | Avg: 1h 16m | Max: 1h 16m
  • 🟩 python: Pass: 100%/1 | Total: 1h 16m | Avg: 1h 16m | Max: 1h 16m

    🟩 cpu
      🟩 amd64              Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    🟩 ctk
      🟩 12.8               Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    🟩 cudacxx
      🟩 nvcc12.8           Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    🟩 cudacxx_family
      🟩 nvcc               Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    🟩 cxx
      🟩 GCC13              Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    🟩 cxx_family
      🟩 GCC                Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    🟩 gpu
      🟩 rtx2080            Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    🟩 jobs
      🟩 Test               Pass: 100%/1   | Total:  1h 16m | Avg:  1h 16m | Max:  1h 16m
    

👃 Inspect Changes

Modifications in project?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

Modifications in project or dependencies?

Project
CCCL Infrastructure
libcu++
CUB
Thrust
CUDA Experimental
stdpar
+/- python
CCCL C Parallel Library
Catch2Helper

🏃‍ Runner counts (total jobs: 1)

# Runner
1 linux-amd64-gpu-rtx2080-latest-1

@leofang
leofang merged commit 7ea0812 into NVIDIA:main Mar 24, 2025
@github-project-automation github-project-automation Bot moved this from In Review to Done in CCCL Mar 24, 2025
@oleksandr-pavlyk
oleksandr-pavlyk deleted the correct-segmented-reduce-tests branch March 24, 2025 22:53
caugonnet pushed a commit to caugonnet/cccl that referenced this pull request Mar 25, 2025
* Fix logic in test_segmented_reduce, also test over different types of offsets

To resolve NVIDIAgh-4197, use `cupy.cumsum` to accumulate over random partition
sizes to form correct offsets sequence.

Add assertions to verify that `offsets` is a non-decreasing sequence, and
that its last element equals the size of the input array.

Perform the test for several plausible offset data types.

* Changes per PR review comments

1. Use `cupy.random` to draw random sample on GPU, rather than on CPU
   followed by a transfer
2. Use `cp.empty` to allocate output, rather than `cp.zeros`
caugonnet added a commit that referenced this pull request Mar 26, 2025
* Enable NVHPC in CUDASTF CI

* Use nvtx_range in the POTRI example

* no need to include nvtx3 anymore here

* Do not use nvtx3 directly

* fix compilation

* WIP: try to pass the type of the execution place to the parallel_for_scope

* pass the exec_place type to parallel_for_scope

* remove a device annotation for a host only lambda

* is_shape_of_v is not working

* experiment to see if we can avoid generating device code in parallel_for with a host place

* fixes for a previous conflict merge

* disable an invalid test

* better typing for ctx.parallel_for with a partition

* use ::std and more types for parallel_for

* Add transform c parallel implementation (#4048)

* Enable setting _CUB_HAS_TRANSFORM_UBLKCP as flag

* Stale comment

* Update make_kernel_user_binary_operator to accept lhs/rhs types

* Add transform c.parallel implementation

* Add tests for c.parallel transform

* Use launcher factory to query ptx version

* Refactor how we get iterator names

* Add comment explaining why we need -default-device

* Address remaining review feedback

* Undo change to make_iterator_kernel_arg

* Restore the comment

* Restore comment

---------

Co-authored-by: Ashwin Srinath <shwina@users.noreply.github.com>

* Drop duplicated system header blocks (#4245)

Those were accidentally duplicated

* Exclude sm101 from RDC testing. (#4247)

* Make `cuda::stream_ref` constructible on device (#4243)

* Make `cuda::stream_ref` constructible on device

There is no reason we should not be able to construct it or extract the pointer out of it on device.

* Pass by value

* Make nvbench work

* Fix logic in test_segmented_reduce (#4198)

* Fix logic in test_segmented_reduce, also test over different types of offsets

To resolve gh-4197, use `cupy.cumsum` to accumulate over random partition
sizes to form correct offsets sequence.

Add assertions to verify that `offsets` is a non-decreasing sequence, and
that its last element equals the size of the input array.

Perform the test for several plausible offset data types.

* Changes per PR review comments

1. Use `cupy.random` to draw random sample on GPU, rather than on CPU
   followed by a transfer
2. Use `cp.empty` to allocate output, rather than `cp.zeros`

* Add new `WarpReduce` overloadings (#3884)

Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>

* Fix #4250 (#4251)

* Refactor fp masks (#4246)

* Implement `views::all` (#4244)

* Implement `views::all`

* [cudax] incorporate P3557 (constexpr completion signatures) into µstdex (#3841)

* incorporate P3557 (constexpr completion signatures) into ustdex

* remove the need for nvcc-specific workarounds

* review feedback

* tweaks to ustdex for clangd

* fix typo

* use concepts portability macros for C++17 support

* Add fixed size segmented reduce (#3969)

* Adds fixed size segmented reduce

* remove unnecessary changes

* revert changes

* Properly deal with parallel_for overloads with grids, add an is_host() for exec_place

* Remove test doing illegal things, and which is not really useful anymore

---------

Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <shwina@users.noreply.github.com>
Co-authored-by: Michael Schellenberger Costa <miscco@nvidia.com>
Co-authored-by: Allison Piper <alliepiper16@gmail.com>
Co-authored-by: Oleksandr Pavlyk <21087696+oleksandr-pavlyk@users.noreply.github.com>
Co-authored-by: Federico Busato <50413820+fbusato@users.noreply.github.com>
Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>
Co-authored-by: David Bayer <48736217+davebayer@users.noreply.github.com>
Co-authored-by: Eric Niebler <eniebler@nvidia.com>
Co-authored-by: Srinivas Yadav <43375352+srinivasyadav18@users.noreply.github.com>
davebayer pushed a commit to davebayer/cccl that referenced this pull request Apr 7, 2025
* Fix logic in test_segmented_reduce, also test over different types of offsets

To resolve NVIDIAgh-4197, use `cupy.cumsum` to accumulate over random partition
sizes to form correct offsets sequence.

Add assertions to verify that `offsets` is a non-decreasing sequence, and
that its last element equals the size of the input array.

Perform the test for several plausible offset data types.

* Changes per PR review comments

1. Use `cupy.random` to draw random sample on GPU, rather than on CPU
   followed by a transfer
2. Use `cp.empty` to allocate output, rather than `cp.zeros`
davebayer added a commit to davebayer/cccl that referenced this pull request Apr 7, 2025
* Enable NVHPC in CUDASTF CI

* Use nvtx_range in the POTRI example

* no need to include nvtx3 anymore here

* Do not use nvtx3 directly

* fix compilation

* WIP: try to pass the type of the execution place to the parallel_for_scope

* pass the exec_place type to parallel_for_scope

* remove a device annotation for a host only lambda

* is_shape_of_v is not working

* experiment to see if we can avoid generating device code in parallel_for with a host place

* fixes for a previous conflict merge

* disable an invalid test

* better typing for ctx.parallel_for with a partition

* use ::std and more types for parallel_for

* Add transform c parallel implementation (NVIDIA#4048)

* Enable setting _CUB_HAS_TRANSFORM_UBLKCP as flag

* Stale comment

* Update make_kernel_user_binary_operator to accept lhs/rhs types

* Add transform c.parallel implementation

* Add tests for c.parallel transform

* Use launcher factory to query ptx version

* Refactor how we get iterator names

* Add comment explaining why we need -default-device

* Address remaining review feedback

* Undo change to make_iterator_kernel_arg

* Restore the comment

* Restore comment

---------

Co-authored-by: Ashwin Srinath <shwina@users.noreply.github.com>

* Drop duplicated system header blocks (NVIDIA#4245)

Those were accidentally duplicated

* Exclude sm101 from RDC testing. (NVIDIA#4247)

* Make `cuda::stream_ref` constructible on device (NVIDIA#4243)

* Make `cuda::stream_ref` constructible on device

There is no reason we should not be able to construct it or extract the pointer out of it on device.

* Pass by value

* Make nvbench work

* Fix logic in test_segmented_reduce (NVIDIA#4198)

* Fix logic in test_segmented_reduce, also test over different types of offsets

To resolve NVIDIAgh-4197, use `cupy.cumsum` to accumulate over random partition
sizes to form correct offsets sequence.

Add assertions to verify that `offsets` is a non-decreasing sequence, and
that its last element equals the size of the input array.

Perform the test for several plausible offset data types.

* Changes per PR review comments

1. Use `cupy.random` to draw random sample on GPU, rather than on CPU
   followed by a transfer
2. Use `cp.empty` to allocate output, rather than `cp.zeros`

* Add new `WarpReduce` overloadings (NVIDIA#3884)

Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>

* Fix NVIDIA#4250 (NVIDIA#4251)

* Refactor fp masks (NVIDIA#4246)

* Implement `views::all` (NVIDIA#4244)

* Implement `views::all`

* [cudax] incorporate P3557 (constexpr completion signatures) into µstdex (NVIDIA#3841)

* incorporate P3557 (constexpr completion signatures) into ustdex

* remove the need for nvcc-specific workarounds

* review feedback

* tweaks to ustdex for clangd

* fix typo

* use concepts portability macros for C++17 support

* Add fixed size segmented reduce (NVIDIA#3969)

* Adds fixed size segmented reduce

* remove unnecessary changes

* revert changes

* Properly deal with parallel_for overloads with grids, add an is_host() for exec_place

* Remove test doing illegal things, and which is not really useful anymore

---------

Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <shwina@users.noreply.github.com>
Co-authored-by: Michael Schellenberger Costa <miscco@nvidia.com>
Co-authored-by: Allison Piper <alliepiper16@gmail.com>
Co-authored-by: Oleksandr Pavlyk <21087696+oleksandr-pavlyk@users.noreply.github.com>
Co-authored-by: Federico Busato <50413820+fbusato@users.noreply.github.com>
Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>
Co-authored-by: David Bayer <48736217+davebayer@users.noreply.github.com>
Co-authored-by: Eric Niebler <eniebler@nvidia.com>
Co-authored-by: Srinivas Yadav <43375352+srinivasyadav18@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Correct test_segmented_reduce.py::test_segmented_reduce

3 participants