WIP: Cuda Bindings

[jclaessens97]

I started the implementation of the Cuda bindings by implementing the bindings described here. thisPR is a work in progress SO DON'T MERGE YET, but just so you can follow along and give some input if needed.

Can you review this small part & tell me if there could be any improvements?

To start, I already have 2 questions:

1. I was wondering why the generateAPITests isn't working on the functions I have to pass an argument in. The test fails because a wrong exception is thrown, although the argument is a required int. The test only succeeds if I write a manual test and set the binding in core.cc also manually instead of using FF_ARG_INT. Any idea?

2. I didn't implement deviceSupports yet because it's a bit unclear to me how I should handle enums (FeatureSet in this case). I took a look @ TermCriteria, but I still don't get it fully because that one has a constructor but FeatureSet doesn't seem to have a ctor.

Cheers!

[justadudewhohacks]

Hi,

The way you implemented the bindings looks fine to me.

I was wondering why the generateAPITests isn't working on the functions I have to pass an argument in. The test fails because a wrong exception is thrown, although the argument is a required int. The test only succeeds if I write a manual test and set the binding in core.cc also manually instead of using FF_ARG_INT. Any idea?

Hmm, could you show me the exception that is thrown? Maybe you forgot to set the usesMacroInferno option in generateAPITests:

opencv4nodejs/test/utils/defaultTests.js

Lines 30 to 32 in dcaf4c5

	// provide backwards compatibility for bindings implemented with
	// macro-inferno macros
	usesMacroInferno = false

I know this is kind of ugly, but in order to test bindings using the FF macros from macro inferno this has to be set. Actually we wanted to move away from macro inferno and use the worker pattern and rather use converters from native-node-utils. But for now I would say, it is ok to keep your implementation of the bindings as is.

2. I didn't implement deviceSupports yet because it's a bit unclear to me how I should handle enums (FeatureSet in this case). I took a look @ TermCriteria, but I still don't get it fully because that one has a constructor but FeatureSet doesn't seem to have a ctor.

I think you can export the featureSet constants simply like the other constants, for example the cv.statModel constants here

[jclaessens97]

This is the error i'm getting:

1) core - cuda
       printCudaDeviceInfo
         sync
           should throw if no args:
     AssertionError: expected '' to include 'expected arg 0 to be'
      at assertError (utils/testUtils.js:13:10)
      at Context.it (utils/testUtils.js:53:5)

  2) core - cuda
       printCudaDeviceInfo
         sync
           should throw if required arg invalid:
     AssertionError: expected '' to include 'GetDevice - expected arg 0 to be of type'
      at assertError (utils/testUtils.js:13:10)
      at expectError (utils/defaultTests.js:112:7)
      at Context.it (utils/defaultTests.js:127:9)



npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! [email protected] test: `mocha --require ./globals --timeout 2000 --recursive ./tests`
npm ERR! Exit status 2
npm ERR! 
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/jeroen/.npm/_logs/2019-04-24T17_36_55_124Z-debug.log

For every generateAPI test (on the methods I implemented) where I need to pass required args.

This is the snippet of the test:

  describe('printCudaDeviceInfo', () => {
    it('should have function printCudaDeviceInfo', () => {
      expect(cv).to.have.property('printCudaDeviceInfo').to.be.a('function');
    });

    generateAPITests({
      getDut: () => cv,
      methodName: 'getDevice',
      hasAsync: false,
      getRequiredArgs: () => [1],
      expectOutput: res => expect(res).to.be.a('number'), 
      usesMacroInferno: true,
    });

and of the binding (like I had it before the commit):

NAN_METHOD(Core::PrintCudaDeviceInfo) {
  FF_METHOD_CONTEXT("Core::PrintCudaDeviceInfo");
  FF_ARG_INT(0, int device);

  // if (!FF_IS_INT(info[0])) {
  //   return Nan::ThrowError("Core::PrintCudaDeviceInfo expected arg0 to an int");
  // }

  // int32_t deviceNum = FF_CAST_INT(info[0]);
  cv::cuda::printCudaDeviceInfo(device);
}

It is probably because I don't check if I receive the argument like I do in the commented part, but I'm wondering why this works without checking:

NAN_METHOD(Imgproc::Canny) {
	FF_METHOD_CONTEXT("Canny");

	FF_ARG_INSTANCE(0, cv::Mat dx, Mat::constructor, FF_UNWRAP_MAT_AND_GET);
	FF_ARG_INSTANCE(1, cv::Mat dy, Mat::constructor, FF_UNWRAP_MAT_AND_GET);
	FF_ARG_NUMBER(2, double threshold1);
	FF_ARG_NUMBER(3, double threshold2);
	FF_ARG_BOOL_IFDEF(4, bool L2gradient, false);

	FF_OBJ jsMat = FF_NEW_INSTANCE(Mat::constructor);
	cv::Canny(dx, dy, FF_UNWRAP_MAT_AND_GET(jsMat), threshold1, threshold2, L2gradient);

	FF_RETURN(jsMat);
}

I'm pretty certain that the treshold parameters are required as well?

[justadudewhohacks]

I think you have a small typo there: methodName: 'getDevice'. GetDevice does not seem to have any required arguments, that's why you are receiving this error.

[jclaessens97]

The error I was talking about was different before, but the 'usesMacroInferno' fixed it. Sorry.

[jclaessens97]

I finished the basic cuda bindings, next up I'm gonna start implementing gpuMat and imgproc, probably this weekend, but not sure yet :)

If you have any remarks let me know.

[goulash1971]

@jclaessens97 wondering whether you're still working on this / how far you managed to get with the bindings ... would love to help if I can :)

[jclaessens97]

@goulash1971 hey, sorry for the late response. Sadly I didn't get much more than just some basics working. I tried to become more familiar with the code and implemented some basic CUDA stuff, but since I'm still going to college where I have some projects, I have an internship and I work at a startup I don't have any time left to work on this at this time. I would like to pick it up later, but for now feel free to start wherever you want :) I did a PR on the other project to add the CUDA flags to the install package.

The commits I added is basically everything I have achieved until now.