torch-mlir icon indicating copy to clipboard operation
torch-mlir copied to clipboard

[linalg] : Use (-)realmax instead of (-)inf to avoid usage of non-finites.

Open sahas3 opened this issue 1 month ago • 4 comments

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.

sahas3 avatar Nov 03 '25 01:11 sahas3

I'm not sure how much this practically matters, but min(x, inf) is only the same as min(x, realmax) if x is 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 for torch.min and the input was a splat torch.inf tensor, then pytorch would return inf and not realmax.

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.

sahas3 avatar Nov 06 '25 19:11 sahas3

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.

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!

sahas3 avatar Nov 25 '25 16:11 sahas3

Looks ok to me.

sjarus avatar Dec 09 '25 06:12 sjarus

Yeah, I am okay with a flag to disable non-finites.

zjgarvey avatar Dec 15 '25 22:12 zjgarvey