Refactor JIT LTO kernel generation#1812
Conversation
Add an algorithm that computes a matrix product, and add a generic CMake function that uses this algorithm to generate a matrix of kernels with desired parameters
dantegd
left a comment
There was a problem hiding this comment.
Minor comments, otherwise this looks great!
divyegala
left a comment
There was a problem hiding this comment.
I think we can normalize how keys are named in the JSON file. If the key ends with:
_name: Actual name of the template_type: Always used to instantiate the actual kernel function template_type_tag: Template parameters forregisterAlgorithm. The JSON should have the whole tag as a valuetag_type_ui._type_val: Value for the types used in constructor ofregisterAlgorithmas_name+_type_val
It's not entirely clear to me what this would look like, or if it would even be possible with the current algorithm. Please post a suggestion with your specific renaming recommendations for |
divyegala
left a comment
There was a problem hiding this comment.
I think the dev team can normalize a naming convention after this PR is merged!
jameslamb
left a comment
There was a problem hiding this comment.
I didn't look closely at the source files in src/neighbors/ivf_flat, trusting other reviewers to be much more knowledgeable about those than me.
Left a few comments about the general setup though. Most are minor but one is big... are we SURE it's to require a Python interpreter to build libcuvs?
Leaving a non-blocking "Comment" review, I'll come back and approve once those questions are answered.
jameslamb
left a comment
There was a problem hiding this comment.
Approving, but please do consider the other comments I left about the strictness and placement of this Python code.
I already discussed this with @divyegala back in December. One of cuvs's dependencies already requires Python to build, so this isn't introducing a new dependency. Other projects that want this matrix product algorithm may not have the same luxury. Once CMake 4.3 with its |
|
/merge |
Follow-up to #1812 (comment) Authors: - Kyle Edwards (https://github.com/KyleFromNVIDIA) Approvers: - James Lamb (https://github.com/jameslamb) - Divye Gala (https://github.com/divyegala) URL: #1836
Add an algorithm that computes a matrix product, and add a generic CMake function that uses this algorithm to generate a matrix of kernels with desired parameters. Authors: - Kyle Edwards (https://github.com/KyleFromNVIDIA) Approvers: - Dante Gama Dessavre (https://github.com/dantegd) - Divye Gala (https://github.com/divyegala) - James Lamb (https://github.com/jameslamb) - Bradley Dice (https://github.com/bdice) URL: rapidsai#1812
Follow-up to rapidsai#1812 (comment) Authors: - Kyle Edwards (https://github.com/KyleFromNVIDIA) Approvers: - James Lamb (https://github.com/jameslamb) - Divye Gala (https://github.com/divyegala) URL: rapidsai#1836
Add an algorithm that computes a matrix product, and add a generic
CMake function that uses this algorithm to generate a matrix of kernels
with desired parameters.