torch.minimum and torch.maximum return a zero whose sign depends on the tensor length #193781

🐛 Describe the bug

torch.minimum and torch.maximum return a zero whose sign depends on the tensor length, because the scalar path and the vectorized path disagree.

import torch

for n in (7, 8):
    a = torch.full((n,), 0.0)
    b = torch.full((n,), -0.0)
    print(n, torch.minimum(a, b)[0].signbit().item())
# 7 False   (+0.0)
# 8 True    (-0.0)

The length where it flips is one full vector, so it moves with the dtype:

maximum is the same with the arguments swapped, torch.maximum(full(n,-0.0), full(n,0.0)) gives -0.0 short and +0.0 long.

Which one is right

The vectorized answer. numpy is sign aware and order independent, and so is torch.fmin / torch.fmax on CPU:

The scalar path just returns whichever argument came first.

Cause

The scalar lambda uses std::min / std::max, which compare with < and so ignore the sign of zero:

at::vec::minimum on NEON is vminq_f32, which is sign aware, hence the mismatch. fmax_kernel and fmin_kernel right below use std::fmin / std::fmax and get it right, which is why those two ops are consistent.

One thing to check in a fix: on x86 at::vec::minimum is _mm256_min_ps, and MINPS is documented to return the second operand when both are zero. So the vectorized path there should give +0 for min(-0,+0), meaning the answer depends on the architecture as well as the length. I only have arm64 to test on, so that part is from reading the code, not measured.

Related, same shape of problem

amin / amax return the sign of whichever element came first, where numpy is order independent:

import torch, numpy as np
print(torch.tensor([0.0, -0.0]).amin().signbit().item())  # False
print(np.array([0.0, -0.0]).min())                        # -0.0

MPS returns the second argument for all four of minimum, maximum, fmin, fmax, so fmin / fmax are order dependent there too:

import torch
a = torch.zeros(8, device="mps"); b = -torch.zeros(8, device="mps")
print(torch.fmin(a, b)[0].signbit().item(), torch.fmin(b, a)[0].signbit().item())  # True False

Fix

The CPU scalar path is a small change, keep the existing NaN check and pick the signed zero explicitly instead of calling std::min. Glad to send a PR for that if you want it. Whether the x86 vectorized path should get a fixup too is a perf call on a hot op, so I would rather ask than guess.

Found with a differential tester that holds the values fixed and varies the tensor length.

Versions

torch 2.15.0a0, CPU and MPS, macOS arm64.

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @aditew01 @kulinseth @malfet @DenisVieriu97 @jhavukainen @aditvenk @Isalia20