[Tile] Actually rewrite complex for extended floating point types once more - #10717
Conversation
0f8a5b3 to
5079a40
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (80)
🚧 Files skipped from review as they are similar to previous changes (75)
📝 WalkthroughSummary by CodeRabbit
WalkthroughExtended floating-point detection and ChangesExtended complex tile support
Suggested labels: Suggested reviewers: Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/plus_equal_scalar.pass.cpp (1)
45-50: 🎯 Functional Correctness | 🔵 Trivialsuggestion: Run the affected tile tests. Use the relevant CCCL CMake preset and targeted test targets to compile and execute the new
__halfand__nv_bfloat16instantiations. The supplied context does not include build or test results. As per coding guidelines, validate changes with targeted builds and tests using the provided CMake presets. As per path instructions, use.agent/skills/cccl-test/SKILL.mdand its routed references for libcudacxx complex-number tests.Sources: Coding guidelines, Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cf1d5208-a708-426c-864b-2362370af637
📒 Files selected for processing (80)
libcudacxx/include/cuda/std/__cccl/extended_data_types.hlibcudacxx/include/cuda/std/__complex/complex.hlibcudacxx/include/cuda/std/__complex/nvbf16.hlibcudacxx/include/cuda/std/__complex/nvfp16.hlibcudacxx/test/libcudacxx/std/numerics/bit/bit.cast/bit_cast.trivially_copyable.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/cases.hlibcudacxx/test/libcudacxx/std/numerics/complex.number/cmplx.over/arg.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/cmplx.over/conj.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/cmplx.over/imag.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/cmplx.over/norm.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/cmplx.over/pow.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/cmplx.over/proj.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/cmplx.over/real.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/assignment_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/assignment_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/divide_equal_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/divide_equal_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/minus_equal_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/minus_equal_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/plus_equal_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/plus_equal_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/times_equal_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.member.ops/times_equal_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.members/construct.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.members/real_imag.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_divide_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_divide_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_equals_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_equals_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_minus_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_minus_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_not_equals_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_not_equals_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_plus_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_plus_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_times_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/complex_times_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/scalar_divide_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/scalar_equals_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/scalar_minus_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/scalar_not_equals_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/scalar_plus_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/scalar_times_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/stream_input.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/stream_output.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/unary_minus.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.ops/unary_plus.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/acos.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/acosh.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/asin.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/asinh.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/atan.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/atanh.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/cos.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/cosh.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/exp.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/log.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/log10.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/pow_complex_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/pow_complex_scalar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/pow_scalar_complex.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/sin.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/sinh.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/sqrt.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/tan.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.transcendentals/tanh.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.tuple/get.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.tuple/tuple_element_compiles.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.tuple/tuple_size_compiles.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.value.ops/abs.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.value.ops/arg.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.value.ops/conj.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.value.ops/imag.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.value.ops/norm.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.value.ops/polar.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex.value.ops/real.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex/abi_latest.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex/traits.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/complex/types.pass.cpplibcudacxx/test/libcudacxx/std/numerics/complex.number/layout.pass.cpp
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
griwes
left a comment
There was a problem hiding this comment.
The question below, and the direct code suggestion, should be resolved before merging. The other suggestion is up to you.
Tests look good, except apparently you've not updated complex.value.opts/proj.pass.cpp; divide_equal_complex.pass.cpp also has an issue, but that one got cleanly caught by CI.
| __nv_bfloat16 x; | ||
| __nv_bfloat16 y; | ||
| }; | ||
| using __complex_nv_bfloat_repr_t = __complex_fake_nv_bfloat162; |
There was a problem hiding this comment.
Suggestion: I started five separate comments recommending to un-special-case this and all of them failed. I hate it here. Best I can think of is having a separate template<typename T> struct __type_to_storage_vector { using __type = typename __type_to_vector<T>::__type; };, in these tile paths specializing for half and bf16, and then using __type_to_storage_vector throughout instead of the special cases here and in half? But honestly I am not sure if that is at all better. Sigh. Though it does mean we're not typedefing the same name to different types in the different branches.
There was a problem hiding this comment.
I mean there are non different branches, there is either tile mode or not
There was a problem hiding this comment.
What I mean is that there's two separate definitions of __complex_nv_bfloat_repr_t, and we could potentially fold it, but this is very much not a blocker.
…e more This ensures that it works fine, although it has a minor perf regression
5079a40 to
b8b8c7c
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
⏱️ CCCL compile-time benchmark comparison: Public headers compile-time benchResult: 0 regression row(s), 6 improvement row(s) above threshold.
Artifacts: reports and traces Direct file processing
🟢 Direct file processing — Improvements
|
🥳 CI Workflow Results🟩 Finished in 4h 39m: Pass: 100%/115 | Total: 4d 20h | Max: 4h 38m | Hits: 45%/1280044See results here. AI failure analysis1. RAPIDS environment merges incompatible rapids-logger pins · 2 jobsExplanation: Both RAPIDS matrices fail before compilation because the generated Conda environment simultaneously requires incompatible `rapids-logger` 0.2 and 0.3 series. The cugraph/wholegraph matrix additionally exposes CUDA 13.3 compatibility constraints while resolving cuML, but its viable candidates still require `rapids-logger` 0.2 and are blocked by the same 0.3 pin. Evidence: Build RAPIDS (optional) / rmm ucxx raft cuvs cugraph wholegraph, step 6 Root cause: The selected RAPIDS 26.10 development branches contribute dependency files pinned to different major-minor `rapids-logger` series, and the build utility merges them into one unsatisfiable environment. The logs do not attribute each pin to its originating cloned repository, so the exact upstream dependency file must be identified during reproduction; the PR diff only changes libcudacxx code and does not modify this CI configuration. Sources: .github/workflows/build-rapids.yml:67, .github/workflows/build-rapids.yml:68, ci/rapids/post-create-command.sh:38. Suggested next steps: Regenerate each failing matrix environment while adding one RAPIDS repository at a time to identify which dependency files request `rapids-logger==0.2.*` and `==0.3.*`, then align those repositories on one compatible series or temporarily select mutually compatible branch revisions. Verify both sets with `RAPIDS_LIBS='rmm ucxx raft cuvs nvforest cuml' .devcontainer/launch.sh -d -c 13.3 -H rapids-conda -- ./ci/rapids/rapids-entrypoint.sh` and the equivalent command using `RAPIDS_LIBS='rmm ucxx raft cuvs cugraph wholegraph'`. Copy this prompt into a coding agentJobs: |
This ensures that it works fine, although it has a minor perf regression