cc: @msluszniak @benITo47
This issue starts a discussion about the proper way we should handle dynamic input shapes. The current solution is a temporary one that fills the missing ExecuTorch runtime feature with adhoc solution, but seeing the changes required for OCR (#1322) makes it clear to me that we need a much more robust solution. I quickly came up with the following idea and would like your input on this:
- The
SymbolicShape utility should be mirrored in both C++ and TS.
- On both sides the validation should allow for checking:
- static vs range dim (both symbolic and concrete, so validating e.g. shape
{1, "H", RangeDim{.min=0, .max="W", .step=4}} (C++ does it but only for concrete RangeDim, TS doesn't have RangeDims)
- validating an enumeration of shapes, so in C++ it should be possible to write
fromJs(..., {{1, "H", 64}, {1, "H", 128}} (TS has this)
- (optionally) it should also allow cartesian product of dims as a shortcut to explicit enumeration, so e.g.
{1, {64,128}, {64, 128}} <=> {{1, 64, 64}, {1, 64, 128}, ...} but this might introduce very severe difficulties for companion method export and symbolic -> concrete shape decay, so I wouldn't actually implement that
- We need a differentiation between
SymbolicShape and ConcreteShape (in both TS and C++).
type Symbol = number | string;
type SymbolicRangeDim = readonly {min: Symbol, max: Symbol, step?: Symbol}
type SymbolicShape = readonly (Symbol | SymbolicRangeDim)[]
type RangeDim = readonly {min: number, max: number, step?: number}
type Shape = readonly (number | RangeDim)[]
and similar distinction on C++ side.
- C++
fromJs would take an enumeration of SymbolicShape or ConcreteShape to validate the input tensor.
- C++ in model.cpp the
dynamicInputShapes would contain only ConcreteShape for each dynamic method (or even perhaps we could build map inputShapes for all methods (not only dynamic) to unify the C++ API)
getMethodMeta would return ConcreteShape for each method input tensor meta (I don't know what about the output tensor meta in the case of dynamic outputs)
- In TS,
validateModelSchema would now check expected SymbolicShape against returned ConcreteShape by model.getMethodMeta and would return a type-safe ConcreteShape so that non-null assertion operator ! wouldn't be needed.
- The Python export contract should only require one additional companion method
get_dynamic_dims_<method_name>, however I'm not sure how to encode both range dims and enumerated shapes here. Adding auxiliary methods on top of the get_dynamic_.... doesn't seem like a robust solution.
cc: @msluszniak @benITo47
This issue starts a discussion about the proper way we should handle dynamic input shapes. The current solution is a temporary one that fills the missing ExecuTorch runtime feature with adhoc solution, but seeing the changes required for OCR (#1322) makes it clear to me that we need a much more robust solution. I quickly came up with the following idea and would like your input on this:
SymbolicShapeutility should be mirrored in both C++ and TS.{1, "H", RangeDim{.min=0, .max="W", .step=4}}(C++ does it but only for concrete RangeDim, TS doesn't have RangeDims)fromJs(..., {{1, "H", 64}, {1, "H", 128}}(TS has this){1, {64,128}, {64, 128}}<=>{{1, 64, 64}, {1, 64, 128}, ...}but this might introduce very severe difficulties for companion method export and symbolic -> concrete shape decay, so I wouldn't actually implement thatSymbolicShapeandConcreteShape(in both TS and C++).fromJswould take an enumeration ofSymbolicShapeorConcreteShapeto validate the input tensor.dynamicInputShapeswould contain onlyConcreteShapefor each dynamic method (or even perhaps we could build mapinputShapesfor all methods (not only dynamic) to unify the C++ API)getMethodMetawould returnConcreteShapefor each method input tensor meta (I don't know what about the output tensor meta in the case of dynamic outputs)validateModelSchemawould now check expectedSymbolicShapeagainst returnedConcreteShapebymodel.getMethodMetaand would return a type-safeConcreteShapeso that non-null assertion operator!wouldn't be needed.get_dynamic_dims_<method_name>, however I'm not sure how to encode both range dims and enumerated shapes here. Adding auxiliary methods on top of theget_dynamic_....doesn't seem like a robust solution.