[MKL-DNN] Wrong API #608
Comments
|
The C API doesn't offer enough information to do that kind of mapping
automatically. We'd need to do a lot more manually:
https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#mapping-a-declaration-to-custom-code
Or we could provide the overloads as part of a helper class on top that is
less work to maintain than Info.javaText():
https://github.com/bytedeco/javacpp-presets/blob/master/opencv/src/main/java/org/bytedeco/javacpp/helper/opencv_core.java
|
|
I was under the impression that you added these changes by hand: Am I wrong about that? These are auto-generated? As for this actual issue, It seems to me that nothing needs to be added, there are only some methods that should be deleted, and some that should have changes in the first parameter. Like this: public static native @Cast("mkldnn_status_t") int mkldnn_stream_submit(mkldnn_stream stream,
@Cast("size_t") long n, @Cast("mkldnn_primitive_t*") PointerPointer primitives,
@Cast("mkldnn_primitive_t*") PointerPointer error_primitive);should become this: public static native @Cast("mkldnn_status_t") int mkldnn_stream_submit(mkldnn_stream stream,
@Cast("size_t") long n, @Cast("mkldnn_primitive_t*") PointerPointer primitives,
@ByPtrPtr mkldnn_primitive error_primitive);I am not completely sure whether error_primitive should be annotated by @ByPtrPtr as stream is not, but you get the idea. It seems to me nothing more complicated is needed in this case. I'd be happy to help with going through this file and making appropriate changes by hand if that's how it works. I am not sure whether I'm missing something and whether it is really how it works, so I'd need confirmation and further instructions... |
|
Yes, the easiest thing would be to add that overload to the helper.
My changes have not removed anything, only adding new overloads, just not
exactly the ones that you're interested in.
|
|
Yes, and some of the overloads that you added do not fit well with mkldnn's C api. At many places where |
|
I told you, the C API doesn't have that information anywhere, so we have to
do it manually, and you want to help that's great, so I gave you material
to look at for that. Take a look at it and if you have any questions, go
ahead, ask away and I'll answer. Sounds good?
|
|
Well, I have offered that help, read the guide you posted (haven't found the answers relevant to this issue there) and asked away with what I understand are the next steps. I need some handholding there. It is still not clear from your answers whether I got it right with that example that I posted :) |
|
Yes, that's fine. Put it in the The target class is generated yes, you can modify it if you want, but your changes will be overwritten by the parser. It's all documented here: |
|
OK, maybe I should ask more specific questions, that can be answered with yes/no or very brief explanations. The point here is that we are talking about one specific type of a simple change. That I can make without becoming expert in JavaCPP.
public static native @Cast("mkldnn_status_t") int mkldnn_stream_submit(mkldnn_stream stream,
@Cast("size_t") long n, @Cast("mkldnn_primitive_t*") PointerPointer primitives,
@ByPtrPtr mkldnn_primitive error_primitive);why stream does not have @ByPtrPtr, or why error_primitive does (I am not seeking full understanding, just a few simple rules related to this particular example)? The point here is that, while I certainly do not have time and resources to became a JavaCPP expert, the changes that need to be done here are rather simple and straightforward, and I know the mkl-dnn C api enough to do the actual matching of each function. I am willing to do the boring part of work, but I need a skeleton to bootstrap that work. |
You can "bootstrap" from OpenCV, so again, take a look, if you have any questions, let me know. I understand you don't want to learn about JavaCPP, but if no one else does, I will remain the only one to make "small changes" and it's impossible for me to do everything on my own. This one is very low priority since it affects in no way the usability of the wrappers. |
|
Yes, I understand your perspective. Unfortunately, I currently do not have enough time to learn enough JavaCPP to bootstrap this on my own. I'll see to find some time in the future to get back to this... |
|
Since it was missing, I've added a section about helper classes to the mapping recipes, that should help: |
|
Thank you. It probably does help. One thing that I think helps most with such thing is a tutorial-ish type of writing. Having detailed reference is of course essential, but to get to that point one has to have a good mental model how things work. At first, a trivial project with a walk-through post is what helps the most. I see that there is some writing with that in mind on the javacpp page, but there may be too much specific details right away, there is lots of text, but I can't find the actual project that one can clone and start experimenting with some brief hints. On the other hand, the issue here is obviously my lack of time to get into this properly rather than insufficient documentation. You're really doing a great work here! Thank you for the effort. |
|
To start a presets from scratch, the sample project is here:
https://github.com/bytedeco/javacpp-presets/wiki/Create-New-Presets
Where are you expecting to find a link to this page?
|
|
In this case, it is probably the case that I didn't search hard enough. However, looking at the javacpp-presets landing page, it seems to me that the issue is more that the link is burried inside less relevant information (for starters). I believe these essential links deserve to be visually more distinguishable, either by putting a subheading Start from this or Beginner Tutorials or something like that. |
|
Yeah, but then people get confused because in general end users shouldn't be reading them... For those that are actually interested in doing something I think there's now enough information that's easily reachable. It's just a lack of interest in working on such tools. It's not something people are interested in working on unfortunately. |
|
Hmmm. Generally, yes, but there stil are people at least potentially interestested. Sometimes it might be that it seems to them getting started is harder than it actually is... |
|
Well, you know, if there were a lot of people interested, I'd be getting help with documentation, but it's not happening, so that's that :) |
|
I did not say a lot. I just said "more than zero" :) |
|
Yeah, so there are ways to communicate with me (GitHub issues, Google Groups, private email, Gitter channels, Skype, direct phone calls, snail mail, etc) so I can guide them and stuff, try to divide the work, answer their questions, etc, but that's not happening either. People are just not interested in working together in making this tool better! Why do you deny this? Do you know of any other tools out there that come close to JavaCPP's capabilities? I'd like to know about them. I'd go work for the guy that creates something better than JavaCPP, but AFAIK that's not happening either. |
|
This list https://github.com/bytedeco/javacpp-presets/graphs/contributors says that 45 people helped in some way. Granted, most of these contributions might be trivial, but these people were interested in this project at least to some extent and helped a bit... |
|
Yes, there are some people like you that help, sure, and I'm very grateful. There are a lot of happy end users, so I'm happy to keep doing most of the work myself until someone else at Oracle or Google or wherever with some capital realizes the importance of this, or not. What's your point? If you'd like to help in some other ways, go for it, but you said yourself you don't have time. So what exactly are you trying to tell me here? You don't have the time, I don't have the time, so who exactly should be doing all that you want me to do? |
|
Hey, take it easy. I don't have time now, but when I do, I'll try to improve the mkldnn part a bit. I understand your frustration. Big Co. that should really be solving this problem do not care. You are solving problems for people, and they do not realize how much effort goes into it. Well, that's the unfortunate reality of every open-source project. I'm sorry, I didn't intend to lead the discussion in that direction. |
|
I think it's safe to assume that the end users of the presets do not read the README.md file of JavaCPP itself, so I've added a "Getting Started" section there to emphasize the new guide: |
|
I've also added this today, that should help too: |
|
Thanks. Looks very informative and to the point. I am working on some other
things right now but as soon as I return to mkldnn I’ll see if I can help
with specific questions and clarifications.
…On Wed, Sep 26, 2018 at 9:57 AM Samuel Audet ***@***.***> wrote:
I've also added this today, that should help too:
https://github.com/bytedeco/javacpp/wiki/Basic-Architecture
What do you think?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#608 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGdJKFas5Nb-eLO3w1CFJWzk90fRbj9ks5uezOFgaJpZM4WLQkY>
.
|
|
To map things like this more easily, I'm thinking we could augment new Info("mkldnn_primitive_t* error_primitive").pointerTypes("@ByPtrPtr mkldnn_primitive"))BTW, I've written here a blog post about the importance of this work: |
This is a consequence of changes introduced in solving bytedeco/javacpp#251
An example is the method
mkldnn_primitive_create. Previously, there was only one version of this method,mkldnn_primitive_create(mkldnn_primitive result, mkldnn_primitive_desc d, mkldnn_primitive_at_t sources, mkldnn_primitive destinations)The problem with the old method is that it did not support an array of primitives as destinations, so
PointerPointeroverload has been introduced:mkldnn_primitive_create(PointerPointer result, mkldnn_primitive_desc d, mkldnn_primitive_at_t sources, PointerPointer destinations).However, this new overload does not match the underlying C API of the original mkldnn, since it replaced both
mkldnn_primitivearguments withPointerPointer. The original API requiresmkldnn_primitiveas the first argument, andmkldnn_primitiveorPointerPointeras the last argument.The (right) overload that is still missing should be:
mkldnn_primitive_create(mkldnn_primitive result, mkldnn_primitive_desc d, mkldnn_primitive_at_t sources, PointerPointer destinations).Basically, this mismatch is present in all creation methods that were overloaded with this change. The intention of the API is that the
mkldnn_primitivethat is being initialized is always a single object, while other arguments sometimes must supportPointerPointer.The text was updated successfully, but these errors were encountered: