Return a finite value for single-element windows, linspace and logspace

[Arthur031221]

Any caller that passes a window size or sweep count of 1, for example hamming<0>({1}) or linspace(2.0f, 5.0f, 1), gets NaN instead of a value, because the generators divide by size - 1 or count - 1, which is 0.

On the base commit, these assign NaN to a one-element float tensor:

• hanning, hamming, bartlett, blackman and flattop of size 1
• linspace and logspace with a count of 1

After this change a one-point window is 1, and a one-point linspace or logspace is the start value (10^first for logspace), as with numpy.hanning(1) and numpy.linspace(2, 5, 1).

The fix returns 1 early in the five window generators when the size is 1, including the JIT source strings, and keeps the step finite in linspace and logspace when the count is 1. Sizes and counts above 1 take the same code path as before.

Tests added in test/00_operators/GeneratorTests.cu: SingleElementWindows (half, bfloat16, float, double) and SingleElementLinspaceLogspace. I compiled these two tests on their own, outside the CMake test target, against the base headers, where all 5 test instances fail with NaN, and against this change, where all 5 pass. I did not run the rest of the suite or the existing Windows test (it needs pybind11).

I did not build with MATX_EN_JIT. Instead I extracted the five edited JIT source strings from the headers, compiled them with nvcc as device operators and ran them: size 1 gives 1.0 for every window, and size 5 gives the expected symmetric window, for example 0.08 0.54 1.0 0.54 0.08 for Hamming.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Fixes division-by-zero in window and range generators for single-element inputs.

The PR appears safe to merge; no new actionable issue remains.

Summary

The PR makes one-element windows return 1 and one-point linspace and logspace sweeps return their start value. It also adds typed window tests and JIT window coverage.

_{Reviews (3) · Last reviewed commit: "Test singleton windows with explicit flo..."}

[cliffburdick]

/build

[cliffburdick]

Thanks for the PR @Arthur031221! Since this adds conditionals in the data path is there possibly a better way to do this where if we detect the size is 1 we can return something that always generates the same value instead of checking?

[Arthur031221]

On the branch in the data path: I timed each generator with 2^26 float elements on an RTX 5090 (best of 7 runs of 200 launches each, two process runs per tree). hanning takes 159.8 and 229.6 us on 609d4d0 and 159.6 and 224.7 us with this branch; bartlett takes 159.5 and 231.4 us on 609d4d0 and 171.1 and 248.3 us with this branch. The same binary varies by more than 40% between process runs, which is larger than any difference between the two trees, so I cannot show a cost from the compare. At this size the kernels write about 1.68 TB/s (2^26 floats in 159.5 us), so they are limited by memory traffic and this timing does not measure instruction count. I did not find a way to return a constant generator for size 1: the type returned by hanning<0>({n}) is fixed at compile time and n is a runtime value, so the size cannot select a different type.

On the review bot findings, I pushed a follow-up commit that covers the three findings, with one difference noted below.

A one-point logspace still divided last - first by 1, so logspace<0>({1}, -1e38f, 3e38f) got an infinite step, and the range multiplied it by index 0 to give NaN. It now uses a zero step when count is 1, the same as the linspace change already on this branch, and the test includes that call. I ran the same call in a standalone program on an RTX 5090 (sm_120): it prints nan on the PR head (5e3a8da) and 0 with the follow-up.

Passing the output type to the window generators found a regression in my earlier commit: bartlett<0, cuda::std::array<index_t, 1>, matxFp16> stopped compiling, because the one-point early return was typed T and the general expression was not. Before the PR it compiled and produced the right values. I converted the general expression to T, and the one-point bartlett call in the typed test now takes the output type. It covers float, double, half and bfloat16. The other four windows still do not compile for half or bfloat16 with an explicit output type, and they did not on 609d4d0 either (cuda::std::cos is ambiguous in hanning.h, hamming.h and blackman.h, and flattop.h uses a0 in device code), so their one-point calls keep the default float generator.

I also added a one-point JIT test for the five windows. With an empty kernel cache, CUDAJITExecutor returns NaN for all five on 609d4d0 and 1 with the changes. With the one-point return in hanning.h changed to 7, the JIT hanning result is 7.

[cliffburdick]

On the branch in the data path: I timed each generator with 2^26 float elements on an RTX 5090 (best of 7 runs of 200 launches each, two process runs per tree). hanning takes 159.8 and 229.6 us on 609d4d0 and 159.6 and 224.7 us with this branch; bartlett takes 159.5 and 231.4 us on 609d4d0 and 171.1 and 248.3 us with this branch. The same binary varies by more than 40% between process runs, which is larger than any difference between the two trees, so I cannot show a cost from the compare. At this size the kernels write about 1.68 TB/s (2^26 floats in 159.5 us), so they are limited by memory traffic and this timing does not measure instruction count. I did not find a way to return a constant generator for size 1: the type returned by hanning<0>({n}) is fixed at compile time and n is a runtime value, so the size cannot select a different type.

On the review bot findings, I pushed a follow-up commit that covers the three findings, with one difference noted below.

A one-point logspace still divided last - first by 1, so logspace<0>({1}, -1e38f, 3e38f) got an infinite step, and the range multiplied it by index 0 to give NaN. It now uses a zero step when count is 1, the same as the linspace change already on this branch, and the test includes that call. I ran the same call in a standalone program on an RTX 5090 (sm_120): it prints nan on the PR head (5e3a8da) and 0 with the follow-up.

Passing the output type to the window generators found a regression in my earlier commit: bartlett<0, cuda::std::array<index_t, 1>, matxFp16> stopped compiling, because the one-point early return was typed T and the general expression was not. Before the PR it compiled and produced the right values. I converted the general expression to T, and the one-point bartlett call in the typed test now takes the output type. It covers float, double, half and bfloat16. The other four windows still do not compile for half or bfloat16 with an explicit output type, and they did not on 609d4d0 either (cuda::std::cos is ambiguous in hanning.h, hamming.h and blackman.h, and flattop.h uses a0 in device code), so their one-point calls keep the default float generator.

I also added a one-point JIT test for the five windows. With an empty kernel cache, CUDAJITExecutor returns NaN for all five on 609d4d0 and 1 with the changes. With the one-point return in hanning.h changed to 7, the JIT hanning result is 7.

Sounds good. I think there's one more legitimate bot comment.

[coveralls]

Coverage is 93.508% — Arthur031221:fix-single-element-generators into NVIDIA:main. No base build found for NVIDIA:main.

[Arthur031221]

You're right about the remaining type coverage finding. The singleton test now instantiates hanning, hamming, blackman, and flattop with explicit float and double generator types. Its half and bfloat16 output cases use float generators for those four windows: their explicit half and bfloat16 forms fail to compile against the base commit. bartlett remains instantiated with each output type.

The GeneratorTests target builds on this commit, and its five singleton test instances pass on the RTX 5090. A separate check of the same calls also passed for all four output types. In an isolated copy, changing only the double hanning singleton return to 7 made the double assertion fail. I have not run the other tests in the target with this follow-up.