Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FEA] Remove default parameters from detail APIs #12944

Closed
vyasr opened this issue Mar 14, 2023 · 0 comments · Fixed by #12978
Closed

[FEA] Remove default parameters from detail APIs #12944

vyasr opened this issue Mar 14, 2023 · 0 comments · Fixed by #12978
Assignees
@vyasr
Labels
feature request New feature or request

Comments

@vyasr
Copy link
Contributor

vyasr commented Mar 14, 2023

Is your feature request related to a problem? Please describe.
In numerous instances we have found errors in implementations failing to pass a stream or memory resource all the way down the call stack, resulting in, for instance, memory being allocated using the wrong resource. #9854 discusses the benefits of requiring callers of detail APIs to always provide explicit streams, which obviates the possibility for this sort of oversight. We should make the same change for memory resources to ensure that they are always properly propagated down.

Describe the solution you'd like
We should systematically remove all default mr parameters from detail APIs. Since stream and mr are always the last two parameters for every function, this will require removing all other default parameters as well. While this task sounds extensive, in practice the number of non-mr default parameters remaining in detail APIs is quite small (completing #9854 got us most of the way there on other parameters).

See discussion on #9584 and #11967 for more context.
@vyasr vyasr added feature request New feature or request Needs Triage Need team to review and classify labels Mar 14, 2023
This was referenced Mar 17, 2023
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@vyasr vyasr self-assigned this Mar 17, 2023
rapids-bot bot pushed a commit that referenced this issue Mar 20, 2023
@vyasr
Remove default detail mrs: part1 (#12964)
1a98ada 
This is the first PR in a sequence removing default mr parameters in detail APIs. Contributes to #12944.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Mike Wilson (https://github.com/hyperbolic2346)

URL: #12964
rapids-bot bot pushed a commit that referenced this issue Mar 20, 2023
@vyasr
Remove default detail mrs: part7 (#12970)
aeb05bf 
This is the seventh PR in a sequence removing default mr parameters in detail APIs. Contributes to #12944.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Mike Wilson (https://github.com/hyperbolic2346)

URL: #12970
rapids-bot bot pushed a commit that referenced this issue Mar 20, 2023
@vyasr
Remove default detail mrs: part6 (#12969)
565efb7 
This is the sixth PR in a sequence removing default mr parameters in detail APIs. Contributes to #12944.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Yunsong Wang (https://github.com/PointKernel)
  - Jason Lowe (https://github.com/jlowe)

URL: #12969
@vyasr vyasr mentioned this issue Mar 20, 2023
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this issue Mar 21, 2023
@vyasr
Remove default detail mrs: part5 (#12968)
6547d96 
This is the fifth PR in a sequence removing default mr parameters in detail APIs. Contributes to #12944.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - MithunR (https://github.com/mythrocks)

URL: #12968
rapids-bot bot pushed a commit that referenced this issue Mar 22, 2023
@vyasr
Remove default detail mrs: part3 (#12966)
b2a65e8 
This is the third PR in a sequence removing default mr parameters in detail APIs. Contributes to #12944.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)

URL: #12966
rapids-bot bot pushed a commit that referenced this issue Mar 22, 2023
@vyasr
Add developer documentation forbidding default parameters in detail A…
6e41db0 
…PIs (#12978)

Resolves #12944. Also reflows the text to ensure consistent text width.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Yunsong Wang (https://github.com/PointKernel)
  - Mark Harris (https://github.com/harrism)
  - Bradley Dice (https://github.com/bdice)

URL: #12978
rapids-bot bot pushed a commit that referenced this issue Mar 23, 2023
@vyasr
Remove default detail mrs: part4 (#12967)
6966fd5 
This is the fourth PR in a sequence removing default mr parameters in detail APIs. Contributes to #12944.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Nghia Truong (https://github.com/ttnghia)

URL: #12967
rapids-bot bot pushed a commit that referenced this issue Mar 24, 2023
@vyasr
Remove default detail mrs: part2 (#12965)
9fbc249 
This is the second PR in a sequence removing default mr parameters in detail APIs. Contributes to #12944.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Karthikeyan (https://github.com/karthikeyann)

URL: #12965
@bdice bdice removed the Needs Triage Need team to review and classify label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vyasr vyasr

Labels
feature request New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add developer documentation forbidding default parameters in detail APIs vyasr/cudf
2 participants
@vyasr @bdice