Validate LSTM/GRU full-sequence output regardless of the returnSequence flag - #955
Validate LSTM/GRU full-sequence output regardless of the returnSequence flag#955shiyi9801 wants to merge 1 commit into
Conversation
| 1. Let |descriptor| be a new {{MLOperandDescriptor}}. | ||
| 1. Set |descriptor|.{{MLOperandDescriptor/dataType}} to |dataType|. | ||
| 1. Set |descriptor|.{{MLOperandDescriptor/shape}} to a [=list/clone=] of |shape|. | ||
| 1. If [=MLOperandDescriptor/checking dimensions=] given |descriptor| returns false, then [=exception/throw=] a {{TypeError}}. |
There was a problem hiding this comment.
This is a good fix — it ensures all output operand descriptors are validated, which was missing before.
Since this now applies to every computed output descriptor (not just GRU/LSTM), could you mention that scope in the PR title and commit log? A number of operators — conv2d, gemm, matmul, gather, tile, expand, where, quantize/dequantizeLinear, argMin/argMax, and the broadcasting element-wise ops — gain new TypeError paths here, and reviewers should know that going in.
Two gaps the constructor-side check can't reach:
concat constructs the descriptor from first's shape and then mutates shape[axis] to the accumulated size, so the validated shape isn't the final one. It needs a check after the mutation.
reshape copies input's descriptor and sets shape directly without calling the constructor. The element count is already covered by the existing product check, but the byte length / rank limits aren't.
Could you also audit for any other methods that bypass the constructor or change dimensions afterwards?
2 participants
Fix #949