Simplify API context managers and decorators #6189
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
| output_dtype | ||
| def __enter__(self): | ||
| if global_settings.api_depth == 0: | ||
| self.cupy_allocator_cm = cupy_using_allocator(rmm_cupy_allocator) |
There was a problem hiding this comment.
Philosophically, this still proves a question that we have avoided so far: if a user is using cupy native pool, then gets into cuML, cuML allocates things in vanilla or pooled RMM, then at the end of the estimator processing the user ends up with cupy objects allocated by two different allocators!
Besides having to document this very well, doing this here instead of at import time like cuDF does makes this scenario way more plausible. As a matter of fact, I'm very convinced the reason we haven't seen much in the way of errors like this one is because cuDF sets cuPy allocator at import time for us.
| ): | ||
| # Dictionary mapping parameter names to their purpose (e.g. as input, | ||
| # target, etc.) for use in parsing input to a wrapped function | ||
| self.params_to_handle = { |
There was a problem hiding this comment.
Given that we already have the concept of handles in cuML and CUDA, I found this naming very confusing for a good couple of minutes, I wonder what would be better naming here
| """ | ||
| def __init__( | ||
| self, | ||
| input_param='X', |
There was a problem hiding this comment.
We should be thinking of common parameters like weights (classes and samples)
| "self": "self", | ||
| } | ||
| if input_param is not None: | ||
| self.params_to_handle[input_param] = "input" |
There was a problem hiding this comment.
This entry doesn't exist in params_to_handle yet, right? Just checking if I missed it somewhere
TODO (currently in draft)