nGraph Preset initial commit

[EmergentOrder]

Another preset incoming!

So I've managed to get it down to just one error on the jningraph.cpp file:

jningraph.cpp:9879:60: error: ‘position1’ was not declared in this scope
         env->SetLongField(arg1, JavaCPP_limitFID, rsize1 + position1);

Any idea what's going on here? This is in the class NgraphResultVector. There are several other NgraphSOMEVector, defined similarly, which don't seem to have the same issue.

[saudet]

Thanks! The issue might have something to do with recent changes in commit bytedeco/javacpp@f214f5c? But there isn't any buffers used in the class... I guess I'll have to look at this.

[EmergentOrder]

After adding runtime::Tensor and runtime::Backend, now I get 2 position related errors.
Also, we are missing the from<SOME> functions from ngraph::element::Type, because they failed to parse (see element_type.hpp.patch).

[saudet]

BTW, add an entry to .travis.yml to have the build tested at least on Linux.

[EmergentOrder]

Added BackendManager, which raises these new errors:

error: ‘void*’ is not a pointer-to-object type
         ::ngraph::onnxifi::BackendManager::unregister(*ptr0);`

 error: ‘void*’ is not a pointer-to-object type
         rptr = (const ::ngraph::onnxifi::Backend*)&::ngraph::onnxifi::BackendManager::get(*ptr0);

[saudet]

I'm not able to build nGraph with cppbuild.sh, it fails with this error (Fedora 27):

[ 60%] Linking CXX shared library libcodegen.so
/usr/bin/ld: cannot find -ltinfo
collect2: error: ld returned 1 exit status
make[2]: *** [src/ngraph/codegen/CMakeFiles/codegen.dir/build.make:188: src/ngraph/codegen/libcodegen.so] Error 1
make[1]: *** [CMakeFiles/Makefile2:759: src/ngraph/codegen/CMakeFiles/codegen.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[ 61%] Linking CXX shared library ../../libinterpreter_backend.so
[ 61%] Built target interpreter_backend
make: *** [Makefile:152: all] Error 2

Could you first get the cppbuild.sh script working? Also, you're missing the "header" to ensure that it builds under the cppbuild subdirectory, and let's use the download function instead of git as per what we did for ONNX:
https://github.com/bytedeco/javacpp-presets/blob/master/onnx/cppbuild.sh
Thanks for your time on this!

[EmergentOrder]

Followed the instructions for CentOS, on Fedora 27 (in Docker) and I get a successful build (up to the errors I reported above).

Without installing the required packages in Fedora, I got errors before even reaching cannot find -ltinfo, but I guess you have those requirements already. It looks like tinfo is in ncurses.

So I'll make install ncurses in cppbuild.sh, not sure if that's the only thing that I need to add though.

[EmergentOrder]

Works for me now on Fedora 27 without package ncurses-devel.
Working on your end?

[saudet]

Hum, it needs ncurses, ok, that's probably not going to work on Mac anytime soon, but that's alright. Let's see what we can do on Linux first.

[saudet]

A few more things to fix with the build:

• We're building ngraph not for onnx, so let's update that line in cppbuild.sh
• It installs in the $HOME/ngraph_dist directory, let's change that to the cppbuild/<platform> directory

[saudet]

In other news, I've fixed compilation failing with error: ‘position1’ was not declared in this scope in commit bytedeco/javacpp@99835a5. For the other one with onnxBackendID, the problem is that it's an @Opaque type and it's trying to call functions that need its definition with @ByVal. You'll need to either skip those functions, or include a header file with the definition for onnxBackendID.

[EmergentOrder]

Awesome, thanks for the fix.
I'll take a look at onnxBackendID.
What about the parsing fail on the from<X> functions in element_type.hpp (currently patched out, but I will remove the patching so it surfaces, because we need those). Can you take a look? Thanks.

[EmergentOrder]

Actually, it looks like it parses element_type.hpp ok, but then fails on the generated java.

[saudet]

We just need to give them names like:

.put(new Info("ngraph::element::from<char>").javaNames("fromChar"));
...

See https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#specifying-names-to-use-in-java and https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#creating-instances-of-c-templates

BTW, since it appears to install everything properly in the install path, we probably don't need to copy everything manually.

[saudet]

Sure thing, fixed in commit bytedeco/javacpp@2cf75e5!

[EmergentOrder]

The last build failed, not on parsing this time, but on the generated Java.

[saudet]

Yes, we need more Info.

[saudet]

The current error shouldn't be hard to resolve, but as usual, let me know if you need help to make progress.

[EmergentOrder]

No problem, I'll take care of that.

Another issue, brought by further changes I haven't checked in yet:
error: cannot convert ‘const Operator* {aka const std::function<ngraph::NodeVector(const ngraph::onnx_import::Node&)>*}’ to ‘ngraph::NodeVector (*)(const ngraph::onnx_import::Node&)’ in assignment

Where I am declaring my own FunctionPointer for Operator.
I tried adding @Const to the declaration, but I get a bunch of other errors related to const/volatile.

This on the generated C++ for this function: const Operator& get_operator(const std::string& name, const std::string& domain) const

Any ideas?

[saudet]

We can't convert an std::function into a function pointer or a function object because the target may not be either a function pointer or a function object. Only creating an std::function from a function pointer or a function object makes sense, not the other way around.

[EmergentOrder]

hmm, ok, looking for an alternative to get_operator,
I found @Namespace("ngraph::onnx_import") public static native @ByVal NgraphFunctionVector load_onnx_model(@Cast("std::istream*") @ByRef Pointer sin);

How can I load a file into a Pointer containing an istream so I can call this?

[saudet]

Enhancing JavaCPP to generate custom call() methods for instances of std::function might become necessary at some point, but I haven't encountered that need yet. It's always possible to define the interface manually anyway. The same for C++ streams, we'd have to map that API first, but it's very inefficient so libraries typically offer alternatives such as reading from a memory buffer or something anyway.

[EmergentOrder]

This is what I had for Operator:

       public static class Operator extends FunctionPointer {
        static { Loader.load(); }
        /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */
        public    Operator(Pointer p) { super(p); }
        protected Operator() { allocate(); }
        private native void allocate();
        public @ByVal native @Cast("ngraph::NodeVector*") Pointer call(@ByRef @Cast("const ngraph::onnx_import::Node*") Pointer a);
   }

so I did define it manually, but got the error when trying to use it.
I'm not clear on how generated custom call() methods would help the cannot convert error I hit. It looked to me like the issue was just not being able to assign non-const <- const in the generated C++.

[saudet]

No, I mean defining mappings for the std::function instance itself, like

@Name("std::function<something>") class SomeFunction {
    @Name("operator()") public native void call(...);
    ...
}

And to access generated types with it, either as part of a helper class or with some Info.javaText:
https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#mapping-a-declaration-to-custom-code
https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#writing-additional-code-in-a-helper-class

[EmergentOrder]

This is ready to merge.
I will likely follow up with separate PR(s) with more headers as needed, particularly when nGraph ONNXIFI support comes.