Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[onnx] update ONNXIFI build output and host dependencies #20112
[onnx] update ONNXIFI build output and host dependencies #20112
Changes from 7 commits
8ee205b
d42ca2f
5114b48
28cec17
f8ec3f5
18d4965
233b72e
94525df
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Features can't "remove" functionality or control alternatives; this should probably be a feature named protobuf which does the full thing and is in the default-features set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. But I think
default-features: ["protobuf"]
can give misunderstanding.Some users may think
onnx[core,pybind11]
is valid and it won't use protobuf.protobuf in
pybind11.dependencies
will look weird.But this one looks weird either in my opinion....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that is unavoidable.
You can add
onnx[core,protobuf]
as a dependency of thepybind11
feature to express dependencies between features.Does using
protobuf-lite
remove the protobuf dependency? If so, the dependency should be moved to the protobuf feature.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add dependency to
onnx[core,protobuf]
for pybind11There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way,
protobuf-lite
also requiresprotobuf
library andptoroc
in its tool.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why delete the dependency
pybind11
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was necessary when I tried to use FimdPython3 with find_package pybind11. But doing that blocks search of NumPy component. Same as the PR note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is, the port needs Python 3 pybind but our pybind11 port is for Python 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. It was quite confusing to me. Let me explain with PR comments...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. commented below. #20112 (comment)
This file was deleted.