bytedeco / javacpp-presets

The missing Java distribution of native C++ libraries
Other
2.68k stars 744 forks source link

[Pytorch, OpenCV] Implicit ArrayRef, Updates for JavaCPP 1.5.10 #1455

Closed HGuillemet closed 10 months ago

HGuillemet commented 11 months ago

Pytorch

OpenCV

saudet commented 10 months ago

Is this pull request done? You're not going to add anything more here for other presets?

HGuillemet commented 10 months ago

Done

saudet commented 10 months ago

Please don't push 200 times a day, Google Drive really doesn't like it, and I don't know how to fix this

HGuillemet commented 10 months ago

I haven't pushed anything for 3 days. Can I help somehow ?

saudet commented 10 months ago

Start by not pushing more than once a day the next time. Why do you do that anyway?

HGuillemet commented 10 months ago

According to the log above, there was just 3 pushes the same day, 4 days ago. No particular reason. I just worked this whole day on the PR I guess. Is the problem due to these 3 pushes a day or that the third one was a forced-push ?

saudet commented 10 months ago

I asked you why do you had to push more than once a day, and you basically answered that you don't need to. So, don't push more than once a day! Ok? Whenever you push something, the builds for everything that you touched get triggered. Don't do that. Maybe I should just disable automatic triggers for pull request if that feature exists (it doesn't seem to exist), but jeez, just don't push more than once a day! Just don't do it

saudet commented 10 months ago

@HGuillemet The SimpleMNIST sample code doesn't work anymore after merging this. Please fix it

java.lang.NullPointerException: Pointer address of argument 0 is NULL.
    at org.bytedeco.pytorch.MNISTBatchDataset.map (Native Method)
    at SimpleMNIST.main (SimpleMNIST.java:82)
    at org.codehaus.mojo.exec.ExecJavaMojo.lambda$execute$0 (ExecJavaMojo.java:283)
    at java.lang.Thread.run (Thread.java:840)
HGuillemet commented 10 months ago

This is the issue about Stack mentioned in bytedeco/javacpp#733. To sum it up: Stack primary template is a declaration without body. Template specializations (that have bodies in this case), are now ignored. So the Java classes mapping template instances are @Opaque with no constructor and new ExampleStack() used in SimpleMNIST returns an object with a null address.

I'll add an ugly javaText to give the text of the whole class and force the generation of a constructor. Unless you have a better idea ?

It could be nice to be able in Info to specify an arbitrary javaText to be appended to a class without having to give the text of the whole class or to hook to another existing, unrelated, declaration in the class.

Shall I push the commit to this PR even if it has already been merged ? or open another ?

saudet commented 10 months ago

I'll add an ugly javaText to give the text of the whole class and force the generation of a constructor. Unless you have a better idea ?

We can just replace the constructor, no?

It could be nice to be able in Info to specify an arbitrary javaText to be appended to a class without having to give the text of the whole class or to hook to another existing, unrelated, declaration in the class.

Yeah, I thought I did that somewhere already, but I guess not. It's not obvious how it would work. We'd probably need to add new flags and what not, so unless we figure out a nice way to do it without that, let's not do that

Shall I push the commit to this PR even if it has already been merged ? or open another ?

Another pull request please, but I'm pretty much done with the release I think, so if no one complained until now I guess no one needs it, so we can do that later I suppose, sounds good?

HGuillemet commented 10 months ago

I'll add an ugly javaText to give the text of the whole class and force the generation of a constructor. Unless you have a better idea ?

Well, in fact I don't know how to replace the whole class text. If I add a javaText to the type, it's copied elsewhere when the type is used, not only in the class definition.

We can just replace the constructor, no?

The constructor is generated here. How would you hook an info to target the constructor ?

Another pull request please, but I'm pretty much done with the release I think, so if no one complained until now I guess no one needs it, so we can do that later I suppose, sounds good?

As you wish. No idea if ExampleStack and TensorExampleStack is of common use. I don't use the dataset api myself. See also the JavaCPP issue 740 I posted some minutes ago and if you want to include something about it before the release.

saudet commented 10 months ago

No, I'm not going to wait after you fixing things endlessly. You keep breaking things, so if someone complains, please fix it. Otherwise, it's not important I suppose.