[MultiThreshold] Generalize data layouts for node execution#143
[MultiThreshold] Generalize data layouts for node execution#143iksnagreb wants to merge 3 commits intofastmachinelearning:mainfrom
Conversation
|
Instead of relying on data layout strings, how about we switch to an attribute |
See fastmachinelearning/qonnx#143 for the similar generalization applied to QONNX MultiThreshold
Hm, @maltanar contributed some significant improvements to the |
The relevant aspect of the data layout annotation seems to be which axis is labeled as the channel dimension "C": We do not actually have to care about the total number and ordering of the other axes, as long as we can find the index of the "C" axis and swap to have "C" at index 1 for node execution (and swap it back afterwards). Falls back to the default assumption that "C" is at index 1 if there is no layout annotation, which is equivalent to the "NCHW" or "NC" layouts. This is a rather experimental change which might break existing code and is currently still restricted to the well-known 2-, 3- and 4-dimensional layouts.
Note: Only covers data layouts for tensors with less than five axes
iksnagreb
force-pushed
the
feature/generalized_multi_threshold_layouts
branch
from
c0b4534 to
a825dfd
Compare
| # Rearrange the input to the expected (N, C, ...) layout | ||
| orig_shape = v.shape | ||
| output = multithreshold(v, thresholds, out_scale, out_bias, channels_last) | ||
| v = v.swapaxes(cdim, 1) |
There was a problem hiding this comment.
Does this always work? Is it an improvement over using the channels_last parameter?
There was a problem hiding this comment.
For 1d, 2d, 3d and 4d input this is fine and matches the previous behavior. Looking at it now, I just realized that in case there is no fallback (>= 5d) or for 0d, it will try indexing into a None. Though, as far as I am aware, this would not affect any model we are supporting at the moment. Still this should be address. I will add a quick workaround, though the entire layout mechanism is long overdue for a proper rework...
There was a problem hiding this comment.
Is it possible to just use the channels_last flag?
There was a problem hiding this comment.
Not really, channels_last can only differentiate between channels at axis 1 or -1, but we had instances where channels could end up at any index, at least temporarily while transforming the model. Usually we always end up with channels last (anything else cannot really be implemented in FINN at the moment), but to get there we might transition through a few mixed-up layouts, but we still want to be able to execute/simulate this.
4 participants
The relevant aspect of the data layout annotation seems to be which axis is labeled as the channel dimension "C": We do not actually have to care about the total number and ordering of the other axes, as long as we can find the index of the "C" axis and swap to have "C" at index 1 for node execution (and swap it back afterwards).
Falls back to the default assumption that "C" is at index 1 if there is no layout annotation, which is equivalent to the "NCHW" or "NC" layouts.
This is a rather experimental change which might break existing code and is currently still restricted to the well-known 2-, 3- and 4-dimensional layouts.
This PR is based on #92 which is less experimental and should be merged first (#92 also does not risk breaking existing code as it only adds new special cases).