[linalg] : Use (-)realmax instead of (-)inf to avoid usage of non-finites.
This change replaces usage of non-finite value inf with finite value realmax for init value of various max/min operations -- no change in semantics of the ops.
I'm not sure how much this practically matters, but
min(x, inf)is only the same asmin(x, realmax)ifxis finite, so this change will possibly result in incorrect results when the input tensors have non-finites. E.g., if you had an e2e test fortorch.minand the input was a splattorch.inftensor, then pytorch would returninfand notrealmax.
Great point, I hadn't thought of that.
It appears that without this change, if I pass a tensor of all -inf to torch.MaxPool2d the result in the linalg-on-tensors path will be a tensor of all -inf, but in the tosa path it is -realmax. This is due to https://github.com/llvm/llvm-project/blob/d2f75f2fe3261264bb4692368aab64aaafb30f08/mlir/lib/Conversion/TosaToLinalg/TosaToLinalgNamed.cpp#L750
After this change linalg-on-tensors also produces tensor of all -realmax as you've mentioned.
cc-ing @sjarus just to notify this behavior for the tosa path.
It seems wrong to me to knowingly introduce this discrepancy with torch even though the scenario doesn't make sense in practice. One alternative is to introduce a flag, such as SupportNonFinites, through the pass-pipeline (and probably fx.export_and_import) which by default will be True and preserve the current behavior of linalg-on-tensors path. If set to false it'll replace inf with realmax and deviate from torch's behavior. The motivation for us is to honor this behavior https://www.mathworks.com/help/coder/ug/run-time-error-checks-1.html for our product.
One alternative is to introduce a flag, such as
SupportNonFinites, through the pass-pipeline (and probablyfx.export_and_import) which by default will beTrueand preserve the current behavior oflinalg-on-tensorspath. If set tofalseit'll replaceinfwithrealmaxand deviate from torch's behavior.
Hi @zjgarvey and @sjarus any thoughts on this alternative approach to maintain the current behavior for linalg path but provide an option for downstream users to deviate from it if they so desire. Since this will introduce an option in the frontend API I want to reach consensus on the design before diving into it. Thanks!
Looks ok to me.
Yeah, I am okay with a flag to disable non-finites.