-
Notifications
You must be signed in to change notification settings - Fork 48
add support for Operators for Generic target needed in MAGIA (again) #195
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
Open
marchioa
wants to merge
13
commits into
pulp-platform:devel
Choose a base branch
from
FondazioneChipsIT:am/magia-kernels-2
base: devel
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
08eecb0
add support for Elu operator for Generic target
marchioa 760c426
add support for LeakyRelu operator for Generic target
marchioa 21969ec
add support for Selu operator for Generic target
marchioa 85a3fb8
add support for Scatter and ScatterElements operator for Generic target
marchioa def4a39
add support for Col2Im operator for Generic target
marchioa 35aa08c
add support for Resize operator for Generic target
marchioa d25e313
refactor of the generic Scatter
marchioa 64d8ec6
about ConvTranspose: refactor parser, add tests (1D and 2D), add 2D k…
marchioa 043b51a
update CHANGELOG.md
marchioa eaac004
fix ConvTranspose.computeShapes
marchioa 1283458
minor fixes addressing coderabbit review
marchioa a30354a
minor refactor
marchioa 4c9dfbf
change FloatUnaryTemplate to UnaryTemplate and use it for Relu and GE…
marchioa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
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.
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 would
inputNodehave no name?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.
It happens in case of an empty input and that is the case of Resize. Specifically, it takes 4 possible inputs and 2 of them (namely
scalesandsizes) are mutually exclusive and one (roi) is optional, therefore it is very likely to have empty inputs.As an example, if the user provides
sizesas an input he must leavescalesempty.To make Resize work I needed to add that guard.
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.
AFAIK, if an input is optional and not specified them the node simply does not have this input. The number of inputs and the position of each input in this list can be used to infer which inputs are provided, which works if a single input is optional (like bias in conv), but it is indeed problematic if you have 3 optional inputs (how to know which is which via position only...).
I guess ONNX is aware of this issue and generates empty inputs to address it. However, I am opposed to supporting unnamed tensors/buffers. Could you have a small context-agnostic pass that names unnamed tensors? My motivation is that not being able to access a tensor/buffer via a key is an issue at many stages of the flow, and I see many cases where it will be a footgun.
For reference, that is how we parse optional bias in conv: