Add environment docs landing page with essential info#10043
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesDocuments CUB execution environments, including construction, composition, configurable properties, defaults, reuse, and related documentation links. The new page is added to the CUB documentation index, determinism and device-wide API text are updated, tuning constraints are clarified, and a test’s formatting is adjusted. CUB execution environments
Possibly related PRs
Suggested reviewers: Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a7959ff3-e8a4-4b6d-af2a-67e6ba08a857
📒 Files selected for processing (3)
docs/cub/determinism.rstdocs/cub/environment.rstdocs/cub/index.rst
| result.begin(), | ||
| cuda::std::less{}, | ||
| cuda::execution::tune(MergePolicySelector{})); | ||
|
|
There was a problem hiding this comment.
it's on purpose. it is very unreadable in the example block chunk. Unless there is a specific reason I would like to keep it.
bernhardmgruber
left a comment
bernhardmgruber
left a comment
There was a problem hiding this comment.
This is great work!
| auto stream = cuda::stream{cuda::devices[0]}; | ||
| auto pool = cuda::device_default_memory_pool(cuda::devices[0]); | ||
| auto env = cuda::std::execution::env{cuda::stream_ref{stream}, pool}; |
There was a problem hiding this comment.
I believe this would work as well:
| auto stream = cuda::stream{cuda::devices[0]}; | |
| auto pool = cuda::device_default_memory_pool(cuda::devices[0]); | |
| auto env = cuda::std::execution::env{cuda::stream_ref{stream}, pool}; | |
| auto stream = cuda::stream{cuda::devices[0]}; | |
| auto pool = cuda::device_default_memory_pool(cuda::devices[0]); | |
| auto env = cuda::std::execution::env{std::move(stream), pool}; |
I think this would create a non-copyable environment (since the stream cannot be copied), but it would work with CUB.
There was a problem hiding this comment.
I just tested it. It works after we moved all environment parameters to const& in cub::DeviceReduce.
There was a problem hiding this comment.
the example calls stream.sync() at the end, would that work fine on a moved-on stream?
- drop the term "environment overload" - cross-link the environments page from the device-wide docs - frame determinism as one example of a requirement - move the policy-selector statelessness warning to the tuning page - clarify that a policy selector only applies to algorithms matching its policy type
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4fd10a48-f1a3-461a-822b-e2c45c01eeaf
📒 Files selected for processing (3)
docs/cub/device_wide.rstdocs/cub/environment.rstdocs/cub/tuning.rst
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/cub/environment.rst
gonidelis
changed the title
🥳 CI Workflow Results🟩 Finished in 6h 24m: Pass: 100%/242 | Total: 2d 03h | Max: 48m 43s | Hits: 99%/152220See results here. |
2 participants
fixes #7712
Adds docs landing page for Environments as discussed in #10013 (comment)
It doesn't go into deep details (e.g CPOs, last sub-section offers fertile ground for that). It provides an overview with
a) a strong definition and
b) minimal usable examples on environment features extracted from our real compilable tests
Pics that showcase the generated HTML page: