Skip to content
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

Add ORT C++ API support and sample code; Use DNNL everywhere & enable… #841

Merged
merged 10 commits into from
Feb 19, 2020

Conversation

EmergentOrder
Copy link
Member

… execution provider at runtime

onnxruntime/cppbuild.sh Outdated Show resolved Hide resolved
@saudet
Copy link
Member

saudet commented Feb 13, 2020

Build fails on Mac: https://travis-ci.org/bytedeco/javacpp-presets/jobs/649548227
We should try to get it working without modifying the code as much as possible...

@EmergentOrder
Copy link
Member Author

I agree. Most of the seds were because of parsing fails though.
The line explicit TypeInfo(OrtTypeInfo* p) : Base<OrtTypeInfo>{p} {} for example fails to parse.
If you can fix those, or know another way to make fewer modifications and still have it working, feel free.
For now though, I used a workaround for sed that should fix it for mac os.

@EmergentOrder
Copy link
Member Author

@saudet Thanks for the assist.
Can we make Session::Run take a ValueVector or something similar to support multiple inputs and reflect the fact that both inputs and outputs are Value* in the preallocated version of Run?

@saudet
Copy link
Member

saudet commented Feb 18, 2020

We can add a helper method for that, but it's not like the original C++ API makes that and a lot of other things clear anyway...

@saudet
Copy link
Member

saudet commented Feb 19, 2020

For the Session::Run() method specifically, if we overload it with ValueVector parameters, people may assume that it can resize it to fit the number of outputs, but the underlying function itself doesn't support that, so it would still be confusing to use. Anyway, we can always design at some later point in time an API that is easier to use, and that also does not add any overhead...

@saudet saudet merged commit 232556f into bytedeco:master Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants