felipecrv commented on PR #43542: URL: https://github.com/apache/arrow/pull/43542#issuecomment-2313125290
> In general this looks good but I'm not convinced about the changes to InputType; I don't know of any kernels which take non-CPU data so it seems more reasonable to leave the InputType refactor for a follow up. @bkietz I can leave it to a follow up, but that would force me to put ad-hoc device type checks in the kernel dispatching code instead of doing it in a systematic way. > On the other hand, Users may have implemented their own kernels which _do_ operate on non CPU data in which case this change to InputType is breaking (since those kernels were not defined with the new constructor and default to gracefully rejecting non-CPU data). I'll have to think on this some more These users were already on thin ice: all it takes is a `GetNullCount()` call in kernel dispatching logic to crash. The bright side is that I already give them a chance to avoid the check by passing the device types in the type matcher. This is why I didn't go for definitive "is cpu" checks and instead allowed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
