Remove zero-value special casing in set_element_async to preserve IEEE 754 -0.0

[wjxiz1992]

Description

device_uvector::set_element_async had a zero-value optimization that used cudaMemsetAsync when value == value_type{0}. For IEEE 754 floating-point types, -0.0 == 0.0 is true per the standard, so -0.0 was incorrectly routed through cudaMemsetAsync(..., 0, ...) which clears all bits — including the sign bit — normalizing -0.0 to +0.0.

This corrupts the in-memory representation of -0.0 for any downstream library that creates scalars through RMM (cudf::fixed_width_scalar::set_value → rmm::device_scalar::set_value_async → device_uvector::set_element_async), causing observable behavioral divergence in spark-rapids (e.g., cast(-0.0 as string) returns "0.0" on GPU instead of "-0.0").

Fix

Per the discussion in #2298, remove all constexpr special casing in set_element_async — both the bool cudaMemsetAsync path and the is_fundamental_v zero-detection path — and always use cudaMemcpyAsync. This preserves exact bit-level representations for all types, which is the correct contract for a memory management library that sits below cuDF, cuML, and cuGraph.

set_element_to_zero_async is unchanged — its explicit "set to zero" semantics make cudaMemsetAsync the correct implementation.

Testing

Added NegativeZeroTest.PreservesFloatNegativeZero and NegativeZeroTest.PreservesDoubleNegativeZero regression tests that verify the sign bit of -0.0f / -0.0 survives a round-trip through set_element_async → element. All 122 tests pass locally (CUDA 13.0, RTX 5880).

Closes #2298

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

Made with Cursor

[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.

[coderabbitai]

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Summary by CodeRabbit

• Tests

  • Added tests preserving negative zero for floats and new multi-threaded tests for memory-resource tracking/statistics; added a helper delayed memory-resource for ABA-style testing.

• Refactor

  • Unified asynchronous element-setting to a single consistent memcpy path; enforced stricter tracking checks and reordered upstream deallocation in adaptors.

• Chores

  • CI/scripts, dependency matrices, and pre-commit hooks updated; version/config handling made more dynamic.

Walkthrough

Removed type-specific memset optimizations in element-setting paths so set_element_async/set_value_async always use host-to-device memcpy; added tests preserving IEEE-754 negative zero for float/double; adjusted resource adaptor insertion/deallocation order and added ABA test utilities; multiple CI and dependency updates.

Changes

Cohort / File(s)	Summary
Element-set implementation cpp/include/rmm/device_uvector.hpp, cpp/include/rmm/device_scalar.hpp	Removed fundamental-type memset/zero-specialization and bool-specific paths; set_element_async / set_value_async now consistently use host->device memcpy (docs updated).
Resource adaptors (thread-safety/ordering) cpp/include/rmm/mr/aligned_resource_adaptor.hpp, cpp/include/rmm/mr/tracking_resource_adaptor.hpp, cpp/include/rmm/mr/statistics_resource_adaptor.hpp	Enforced unique insertion checks (try_emplace + RMM_EXPECTS) and moved upstream deallocate calls to after protected updates to avoid ABA/order issues.
New test helpers & MR tests cpp/tests/mr/delayed_memory_resource.hpp, cpp/tests/mr/statistics_mr_tests.cpp, cpp/tests/mr/tracking_mr_tests.cpp	Added delayed_memory_resource to simulate deallocation delays and multithreaded tests for statistics and tracking adaptors to exercise ABA/race scenarios.
Element tests cpp/tests/device_uvector_tests.cpp	Added tests PreservesFloatNegativeZero and PreservesDoubleNegativeZero verifying signbit preservation when setting -0.0 asynchronously; added include and year update.
CI scripts ci/download-torch-wheels.sh, ci/test_python_integrations.sh, ci/test_wheel.sh, ci/test_wheel_integrations.sh, ci/release/update-version.sh	Added wheel-downloader script, changed PyTorch wheel/constraint handling, made PyTorch GPU gating/constraint vars explicit, and removed two sed in-place updates from release script.
Dependencies & config dependencies.yaml, .pre-commit-config.yaml, cpp/examples/versions.cmake	Reworked PyTorch dependency matrices and pins (CUDA-specific wheels), added new public block torch_only, updated pre-commit hook revision and exclusions, and made versions.cmake derive RMM_TAG from rapids_config.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• examples: read tag from RAPIDS_BRANCH file #2293 — overlapping changes to pre-commit hook exclusions and deriving RMM_TAG from rapids_config (matches .pre-commit-config.yaml and cpp/examples/versions.cmake edits).
• ensure 'torch' CUDA wheels are installed in CI #2279 — adds the Torch wheel download workflow and related CI adjustments (matches ci/download-torch-wheels.sh and PyTorch constraint logic).
• Fix ABA problem in tracking resource adaptor and statistics resource adaptor #2304 — implements similar ABA/thread-safety fixes and tests for resource adaptors (matches try_emplace/insertion checks, moved deallocate ordering, delayed_memory_resource, and multithreaded MR tests).

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check	❓ Inconclusive	Multiple supporting changes are included (pre-commit config bumps, CI scripts, dependencies.yaml rewrites, new test utilities), but their necessity to the core fix cannot be fully assessed without additional context on whether these are prerequisites or incidental.	Verify that all changes in CI scripts, dependencies.yaml, pre-commit config, and new test utilities (delayed_memory_resource.hpp) are directly related to or necessary for the -0.0 preservation fix and regression tests.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: removing zero-value special casing in set_element_async to preserve IEEE 754 -0.0 sign bits.
Description check	✅ Passed	The description clearly explains the bug (IEEE 754 -0.0 treated as +0.0), root cause (cudaMemsetAsync clearing sign bits), impact (downstream scalar divergence), and the fix (remove special casing and always use cudaMemcpyAsync).
Linked Issues check	✅ Passed	The PR fulfills the main objectives from #2298: removes zero-value special casing from set_element_async, adds regression tests for -0.0 preservation, and includes supporting documentation updates across multiple files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.

Change the reviews.profile setting to assertive to make CodeRabbit's nitpick more issues in your PRs.

[Copilot]

Pull request overview

Updates rmm::device_uvector::set_element_async to preserve floating-point negative zero (-0.0) by removing the previous zero/bool fast-paths that used cudaMemsetAsync, and adds regression tests to ensure the sign bit is retained.

Changes:

• Remove cudaMemsetAsync-based fast-paths in device_uvector::set_element_async (zero values and bool).
• Add new tests validating that -0.0f and -0.0 preserve their sign bit through set_element_async + element.
• Add <cmath> include and update copyright year.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
cpp/include/rmm/device_uvector.hpp	Removes the memset-based “zero/bool optimization” from set_element_async to avoid clobbering -0.0’s sign bit.
cpp/tests/device_uvector_tests.cpp	Adds regression tests for preserving -0.0 sign bit for float and double.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

🧹 Nitpick comments (1)

cpp/include/rmm/device_scalar.hpp (1)

168-196: Clean up stale memset wording in adjacent comment.

Nice update on Line 168. To keep docs consistent, the deleted-overload comment on Lines 194-195 should also drop "/ memset" since this path is now memcpy-only.

Suggested doc-only diff

-  // literal can be freed before the async memcpy / memset executes.
+  // literal can be freed before the async memcpy executes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cpp/include/rmm/device_scalar.hpp` around lines 168 - 196, The comment next
to the deleted rmm::device_scalar::set_value_async(value_type&&,
cuda_stream_view) overload still mentions "/ memset" which is stale because
set_value_async now uses memcpy-only; update the inline comment above the
deleted overload to remove the "/ memset" phrase so it reads something like
"Disallow passing literals to set_value to avoid race conditions where the
memory holding the literal can be freed before the async memcpy executes."
Ensure you modify the comment text near the deleted overload for set_value_async
to reference only memcpy and the race condition.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cpp/include/rmm/device_scalar.hpp`:
- Around line 168-196: The comment next to the deleted
rmm::device_scalar::set_value_async(value_type&&, cuda_stream_view) overload
still mentions "/ memset" which is stale because set_value_async now uses
memcpy-only; update the inline comment above the deleted overload to remove the
"/ memset" phrase so it reads something like "Disallow passing literals to
set_value to avoid race conditions where the memory holding the literal can be
freed before the async memcpy executes." Ensure you modify the comment text near
the deleted overload for set_value_async to reference only memcpy and the race
condition.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 29a906e9-8ddf-4c5f-81d6-2b92f9f71c47

📥 Commits

Reviewing files that changed from the base of the PR and between fe360fd and 20f799e.

📒 Files selected for processing (1)

• cpp/include/rmm/device_scalar.hpp

[bdice]

/ok to test 20f799e

[bdice]

I will retarget this to release/26.04 so this bugfix can be released sooner.