Content: Specify output size calculations #582
Conversation
|
This is basically a translation of the Chromium C++ implementation into spec-ese, although parameter validation remains separated out. That can be revisited. And like the Chromium implementation, there's a lot of logic sharing among the ops. We could also go further with helper ops, e.g. unpacking and re-packing operand layouts given an MLOperandLayout |
|
Wow, this is big! Thanks! |
Yeah... I didn't know what I was getting myself into. |
| </summary> | ||
| <div class=algorithm-steps> | ||
| 1. Let |effectiveFilterSize| be ( |filterSize| - 1 ) * |dilation| + 1. | ||
| 1. Let |outputSize| be ( |inputSize| - |effectiveFilterSize| + |beginningPadding| + |endingPadding| ) / |stride| + 1. |
There was a problem hiding this comment.
Should we handle any errors? Like |outputSize| is overflow or underflow?
There was a problem hiding this comment.
That's a good question... the specs I've been involved in usually operate in an abstract space with infinite precision, unlimited range, etc., with very limited checking.
I noticed the Chromium prototype impl is using checked math (i.e. testing underflow/overflow), and given the sizes that ML deals with it, it seems like this is going to be a practical concern.
We can spec this any way we want... ideas welcome. We should look at other specs for inspiration, too.
There was a problem hiding this comment.
Hmm, I've always found when implementing specs (like bidi, line breaking, vertical layout...) that I was grateful they left out that degree of fine validation and instead focused on the algorithm itself. Any validation specific to the nature of the operators (e.g. maybe an operator only supports even sizes) belongs in there, but too much otherwise muddies an already complex document. Maybe we can even call that out somewhere in a common section, that the implementation should handle overflow/underflow/safe limit checks, but not as extra prose for each operator.
inexorabletash
force-pushed
the
content-output-shapes
branch
from
f66b135 to
15aabf7
Compare
|
Marking as draft. Once #587 merges I'll push a rebased version. I have it locally and it's much smaller; most of the lines now handle the nchw/nhwc layout unpacking/packing. |
This covers: * Reduction ops (including argMin/argMax) * conv2d * convTranspose2d * Pooling ops (which rely on conv2d) Fixes webmachinelearning#500
Co-authored-by: Ningxin Hu <[email protected]>
Co-authored-by: Ningxin Hu <[email protected]>
inexorabletash
force-pushed
the
content-output-shapes
branch
from
5989deb to
2f9868c
Compare
|
Okay, rebased - sorry about the forced push, but it should be ready for another review now. |
SHA: 69e7ad6 Reason: push, by huningxin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This covers:
Fixes #500
Preview | Diff