Strip invalid dim_param and dim_value values out. Allow re-use in event of shape mismatch if buffer is large enough#1439
Conversation
Allow re-use of a large enough buffer if there's a shape mismatch.
…ate on Monday to change the dim_param name to something that isn't special cased. Allow '0' in dim_value for now due to unit test infrastructure limitations (some tests provide input with 0 as a dimension and we can only create a model with an exact match of the input dimensions).
…param (equivalent to None).
…d dim_value being set.
|
Looks good to me. Recording one of the points discussed offline here for future reference/extension: It is possible to further extend the graph-implementation to create unique symbolic parameter names for each unknown dimension. This can enable more precise shape-inference and, consequently, greater reuse of buffers. However, this does require extra logic to identify all symbolic names used in the graph (as well as sub-graphs) first to ensure uniqueness of generated names. |
onnxruntime/core/graph/graph.cc
Outdated
| auto& dim = *shape->mutable_dim(i); | ||
| if (dim.has_dim_param()) { | ||
| auto dim_param = dim.dim_param(); | ||
| if (dim_param.empty() || dim_param == "None" || dim_param == "-1") { |
There was a problem hiding this comment.
dim_param == "None" [](start = 33, length = 19)
why special case on these strings? I think it's valid from ONNX spec to have any string for dim_param. Besides, I've seen models with "?" in dim_param, which is similar to "None".
There was a problem hiding this comment.
Some ONNX exporters apparently use string like "None". The key problem is that these exporters convert every unknown dimension to the same string "None" (even though the different unknown dimensions may not have the same value at runtime) … this is inconsistent with the ONNX spec. This change is intended to help run those models … otherwise, those models would fail with a runtime error (in cases where there is a dimension mismatch).
Don't check for string '-1' as there's no proven usage of that.
3 participants
Description:
Remove known invalid values from dim_param and dim_value.
Treats the removed values as unknown which allows shape inferencing to fill them in where possible.
Allow re-use of a buffer if there's a shape mismatch and the buffer is large enough.
Motivation and Context
Allows existing models that have invalid dim_value and dim_param values set by converters to still run.
Maintains preventing buffer overflow errors which may silently fail and lead to invalid results.
Warns if there's a shape mismatch which most likely indicates a bad model which is only able to run by chance (50% of the time a shape mismatch will result in a failed attempt to re-use a buffer that is too small)