-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Kernel signature uniformisation #11
Comments
Awesome idea @StRigaud ! Just some questions:
|
Well, the idea is to avoid such specific name tag when we can. Has A down side is that we loose information from the parameter naming, but we gain a heavy generalization on the kernel call, and we can have still this information at the Python or C++ level.
Honestly, I am fine with it. I proposed |
Yeah, that's because I was lazy. However, I think code quality / readability would profit from proper variable names. They still should have some "src" and "dst" in it, because in OpenCL1.2 images are not supported to be read-write on some target platforms.
Yes. And I'm happy to write down these rules. Maybe here would be the right place. |
I don't think we should change the initial An input is an input, whatever is a label image or a distance map. The kernel operation set the context of the input and output. An example is in ITK where all input and output are called by a SetInput and GetOutput methods, whatever are the I/O. Hence I don't think it should have a more specialise name, at least for the main Input and Output. The side input like flags, or label maps could have specific names though as they would concern specific kernels. And we need exception to the rules otherwise my French side will be triggered 😁 |
The same idea apply on the kernel name. Nearly all of them are already fitting a format:
examples:
though some would be modify as |
Here is an excel sheet parsed from all the kernels as:
And for each parameters I subdivided in 3 columns:
I let you enjoy the diversity of @haesleinhuepf imagination at naming stuff. For now, the excel is in view only, we could use it for a base of discussion, highlight issues, and to track modifications. |
TODO for @haesleinhuepf
|
Though most of the implementation follows a certain logic, some signature different and varies, by little or by a lot. Both in parameters name and order. Some example: maximum_z_projection_x, absolute_2d_x, and add_image_and_scalar_2d_x
I believe it could be nice to uniform as much as possible the kernel signature, to make it a generic as possible, by defining a parameter name nomenclature and order. This would simplify certain implementation aspect on the C++ side (and Python?), improve scalability, and facilitate most of the repetitive implementation (auto-generation?). And this could also facilitate the writing of a documentation raised in #10 .
I would be ok to do it, with anyone who also want to suffer with me, but we need first to have a good signature rule. I am not aware enough yet with all the kernels and the specific cases but here are some first suggestions from what I could see:
src
anddst
dst
, and the second parameter to besrc
. This is mainly becausedst
is always present (e.g. set_2d_x)src
,src2
,src3
anddst
,dst2
,dst3
This would lead to this kind of signature:
__kernel do_something_2d ( dst, src )
__kernel do_something_2d ( dst, src, src2, scalar, scalar2 )
And the fun part begins with multiple input and output as it raise two options:
__kernel do_something_2d ( dst, dst2, src, src2, scalar, scalar2 )
__kernel do_something_2d ( dst, src, dst2, src2, scalar, scalar2 )
The text was updated successfully, but these errors were encountered: