diff --git a/.github/scripts/gate_suites.py b/.github/scripts/gate_suites.py index 4340122b..535a3e85 100644 --- a/.github/scripts/gate_suites.py +++ b/.github/scripts/gate_suites.py @@ -120,6 +120,7 @@ "devices/mps/test_mpsattn.py": BOTH, "devices/mps/test_mpsconst.py": BOTH, "devices/mps/test_mpsgen.py": BOTH, + "devices/mps/test_mpsextreme.py": BOTH, "devices/mps/test_mpsfwd.py": BOTH, "devices/mps/test_mpsinplace.py": BOTH, "devices/mps/test_mpsrefuse.py": BOTH, @@ -171,6 +172,7 @@ "numerics/test_int8.py": BOTH, "numerics/test_remeasure2.py": BOTH, "ops/test_canine.py": BOTH, + "ops/test_cmpscalar.py": BOTH, "ops/test_complex.py": BOTH, "ops/test_cplx2.py": BOTH, "ops/test_equal_allclose.py": BOTH, @@ -185,6 +187,7 @@ "ops/test_repeat.py": BOTH, "ops/test_rnn.py": BOTH, "ops/test_scatter.py": BOTH, + "ops/test_signedzero.py": BOTH, "ops/test_strided.py": BOTH, "ops/test_tail1.py": BOTH, "ops/test_tail2.py": BOTH, diff --git a/docs/devices/matrix.md b/docs/devices/matrix.md index 51aceaec..37de091d 100644 --- a/docs/devices/matrix.md +++ b/docs/devices/matrix.md @@ -643,23 +643,18 @@ the source text. Run now, in `test_the_nan_seed_call_sites_answer_where_they_are #### A silently wrong answer found on the way, older than this branch -**`aten.amax.default` on `mps` drops NaN.** Over `[1.25, nan, 0.5, 3.75]` it -returns `3.75` where upstream returns `nan`; the `cpu` path returns `nan` -correctly. The cause is `aten.rs::amax_keepdim_anywhere`, which sends a Metal -tensor to candle's `max_keepdim`, the NaN-skipping fold that the docstring -above `nan_along_dim` calls "the third repair of one predicate and ... meant -to be the last". **It is the fourth.** - -It is **not** this round's change and **not** cause D: the function is -byte-identical in `7457147`'s `aten.rs`, so it predates both this branch and -the int8 round. It is left unfixed deliberately. It is a different operator -family from the one this round was sent to verify, and the same helper is also -called from the softmax row-max, so the blast radius wants its own round. -`test_the_nan_seed_on_mps_is_either_refused_or_a_recorded_defect` **pins** it: -it asserts the wrong answer is still the wrong answer, and goes RED the moment -somebody repairs the operator, which forces the agreement assertion back in. -This is the `clamp` shape of §7.9 again, found the same way, by giving a cell -an input its published shape never had. +**`aten.amax.default` on `mps` used to drop NaN; fixed by issue #49.** Over +`[1.25, nan, 0.5, 3.75]` it returned `3.75` where upstream returns `nan`; the +`cpu` path returned `nan` correctly. The cause was +`aten.rs::amax_keepdim_anywhere`, which sent a Metal tensor to candle's +`max_keepdim`, the NaN-skipping fold that the docstring above `nan_along_dim` +calls "the third repair of one predicate and ... meant to be the last". It was +the fourth, and it predated this branch and the int8 round. Issue #49 made it +propagate NaN and give `-inf` for all `-inf` rows. It is now graded against +upstream on `mps` in +`test_the_nan_seed_call_sites_answer_where_they_are_reachable`, and +`test_the_nan_seed_on_mps_max_dim_and_min_dim_agree_with_upstream` holds the +other two. #### The bracket, and the host-twin experiment run on Metal @@ -837,8 +832,8 @@ it is proposed rather than taken. - - + + ### 4.3 470 refusals still hand back a candle symbol diff --git a/docs/numerics/DTYPEDEV.md b/docs/numerics/DTYPEDEV.md index 68cb1877..e8c47026 100644 --- a/docs/numerics/DTYPEDEV.md +++ b/docs/numerics/DTYPEDEV.md @@ -268,6 +268,12 @@ Two rows are worth reading carefully: The sixteen are in `_MPS_REACHES` and are graded by value against upstream by `test_the_dtype_device_matrix_agrees_with_upstream`, which is what makes `16` a claim about numbers rather than about dispatches. +* **`eq` on `mps` for `float32`, `float16` and `bfloat16` joined `_MPS_REACHES` + with issue #49.** Every float comparison on Metal used to die with + `Metal contiguous to_dtype F32 F64 not implemented`, because it widened to + `f64` first and Metal has no `f64`. It now computes in the operand dtype, and + the three cells are graded by value against upstream by + `test_the_dtype_device_matrix_agrees_with_upstream`. ### 3.2 Dtypes this build cannot store at all diff --git a/tests/devices/mps/test_constset.py b/tests/devices/mps/test_constset.py index 6680aa37..c54e86e5 100644 --- a/tests/devices/mps/test_constset.py +++ b/tests/devices/mps/test_constset.py @@ -707,6 +707,21 @@ def test_the_nan_seed_call_sites_answer_where_they_are_reachable(): _same(a, b) for a, b in zip(values, expect)): bad.append("%s on cpu gave %r, upstream gave %r" % (key, values, expect)) + # amax on mps (issue #49): it used to return the largest non-NaN element + # (candle's NaN-skipping Metal fold); it now propagates NaN like upstream. + if _mps_or_skip("the amax NaN seed on mps") is not None: + for dt in _DTYPES: + expect = want["amax/%s" % dt]["values"] + try: + values = _ask_nan("amax", dt, on_mps=True) + except Exception as e: # noqa: BLE001 -- the refusal is the finding + bad.append("amax/%s on mps raised %s: %s" + % (dt, type(e).__name__, str(e).splitlines()[0])) + continue + if len(values) != len(expect) or not all( + _same(x, y) for x, y in zip(values, expect)): + bad.append("amax/%s on mps gave %r, upstream gave %r" + % (dt, values, expect)) assert saw_a_nan, ( "not one upstream answer in this table is NaN, so the seed these call " "sites exist for was never reached and this test proves nothing. Put " @@ -715,41 +730,24 @@ def test_the_nan_seed_call_sites_answer_where_they_are_reachable(): + "\n ".join(bad)) -def test_the_nan_seed_on_mps_is_either_refused_or_a_recorded_defect(): - """What the `mps` half of those call sites actually does, measured. - - Two different answers, and only one of them is acceptable: - - * `max.dim` and `min.dim` **agree with upstream** on `mps` since issue #29: - they no longer read back (the index is `first_extremum_index`, computed on - the device, and the value is gathered at it), so `nan_shaped_like` is - not on their path and a NaN in the slice comes out as the NaN element. - Asserted element-wise against upstream, NaN as NaN. - - * `aten.amax.default` **answers, and answers wrongly.** With a NaN in the - input it returns the largest non-NaN element where upstream returns NaN. - This is not cause D and it is not this round's change: the cause is - `aten.rs::amax_keepdim_anywhere`, which routes a Metal tensor to - candle's `max_keepdim` -- the NaN-skipping fold that the docstring above - `nan_along_dim` calls "the third repair of one predicate and ... meant - to be the last". It is the fourth, it predates both this branch and the - int8 round (byte-identical in `7457147`'s `aten.rs`), and the same - helper is called from the softmax row-max, so the blast radius is wider - than `amax`. - - **This test pins the defect rather than hiding it.** It asserts the wrong - answer is still the wrong answer, so the moment somebody repairs - `amax_keepdim_anywhere` this test goes RED and forces them here to replace - it with the agreement assertion. A defect nobody has written down is - rediscovered; a defect with a failing test is a decision. Deleting this - test without fixing the operator puts it back in the first category. +def test_the_nan_seed_on_mps_max_dim_and_min_dim_agree_with_upstream(): + """`max.dim` and `min.dim` on `mps`, measured against upstream. + + They agree since issue #29: they no longer read back (the index is + `first_extremum_index`, computed on the device, and the value is gathered + at it), so `nan_shaped_like` is not on their path and a NaN in the slice + comes out as the NaN element. Asserted element-wise, NaN as NaN. + + `aten.amax.default` used to be pinned here as a recorded defect (it + returned the largest non-NaN element on mps). Issue #49 fixed it, and it + is now graded in + `test_the_nan_seed_call_sites_answer_where_they_are_reachable`. """ device = _mps_or_skip("the NaN seed on mps") if device is None: return want = _nan_oracle(_NAN_OPS) bad = [] - for case in ("max_dim", "min_dim"): for dt in _DTYPES: expect = want["%s/%s" % (case, dt)]["values"] @@ -763,28 +761,6 @@ def test_the_nan_seed_on_mps_is_either_refused_or_a_recorded_defect(): _same(a, b) for a, b in zip(values, expect)): bad.append("%s/%s answers %r on mps, upstream answers %r" % (case, dt, values, expect)) - - for dt in _DTYPES: - expect = want["amax/%s" % dt]["values"] - assert any(v != v for v in expect), ( - "upstream's amax/%s over %r is no longer NaN, so this pin is " - "measuring nothing" % (dt, _NAN_SOURCE)) - values = _ask_nan("amax", dt, on_mps=True) - if all(_same(a, b) for a, b in zip(values, expect)): - bad.append( - "amax/%s on mps now agrees with upstream (%r). The recorded " - "defect in amax_keepdim_anywhere appears to be FIXED -- that " - "is good news: delete this loop and move amax into " - "test_the_nan_seed_call_sites_answer_where_they_are_reachable " - "on mps as well." % (dt, values)) - elif values != [max(v for v in _NAN_SOURCE if v == v)]: - bad.append( - "amax/%s on mps gave %r, which is neither upstream's answer " - "%r nor the known NaN-skipping one %r. The defect has changed " - "shape and needs re-diagnosing rather than re-pinning" - % (dt, values, expect, - [max(v for v in _NAN_SOURCE if v == v)])) - assert not bad, ("the mps NaN seed behaviour is not what is recorded:\n " + "\n ".join(bad)) diff --git a/tests/devices/mps/test_mpsextreme.py b/tests/devices/mps/test_mpsextreme.py new file mode 100644 index 00000000..9fc7865b --- /dev/null +++ b/tests/devices/mps/test_mpsextreme.py @@ -0,0 +1,327 @@ +"""Extrema, comparisons and random draws on mps (issue #49). + +Found by #29's design round, on the build #29 landed: + +* `aten.amax.default` on mps answered `-FLT_MAX` (f16 `-65504`, bf16 + `-3.39e38`) for an all-`-inf` input and dropped NaN (`amax([nan, 1])` was + `1.0`). candle's Metal reduce is seeded with the finite lowest value and + skips NaN. `max.default`, `min.default`, `max.dim` and `min.dim` had already + moved to `first_extremum_index` + a gather in #29; `amax` had not, because + it goes through `amax_keepdim_anywhere`, which on Metal was candle's + `max_keepdim` alone. +* Every float comparison on mps (`eq`, `ne`, `lt`, ... in both the `Tensor` + and `Scalar` overloads) and `to(torch.bool)` of a float mps tensor died with + `Metal contiguous to_dtype F32 F64 not implemented`: `compare_common` and + `to_copy_default` widened to `f64` first, and Metal has no `f64`. The issue + named the empty tensor; the matrix (docs/devices/matrix.md §6) shows the + non-empty float cells REFUSES too. +* `torch.randn(..., device="mps")` was reported failing in `aten.normal_` with + `Metal contiguous to_dtype F64 F32`. Reading this tree, `normal_inplace` + narrows its draws on the host (`host_vec`) before the upload, so the report + may predate it; this file measures it either way. + +Grades, in AGENTS.md §16's terms: + +* *agrees* -- every value against upstream torch, in a separate subprocess + that asserts it imported upstream. **The oracle is upstream on the CPU, not + upstream on mps**, and the difference is not hypothetical: measured on torch + 2.13.0 on this Mac, upstream mps answers index `-1` for `max(dim=)` of an + all-`-inf` row and the *last* NaN's index where upstream CPU answers `0` and + the first NaN's (12 of the 72 rows below differ, all of them `max.dim` / + `min.dim` indices; every value and every comparison agrees). The CPU answer + is the documented one (the first maximal index) and is what `generate` + parity in #29 was measured against. Compared exactly, NaN equal to NaN and + the sign of a zero ignored (`amax`'s zero sign is a vectorisation artefact + upstream). +* *ran on the GPU* -- `_metal_counters()` bracketing only the dispatch: + compute encoders > 0 and zero host downloads. A host-computed twin can keep + every value right; it cannot launch a compute encoder, and it has to read + its operand back. +* the random draws are made on the host by this crate's RNG and uploaded + (that is the design: the stream is upstream's CPU stream, docs/numerics/RNG.md), + so their bracket asserts an upload and **no download**, and the values are + checked against this shim's own CPU draws from the same seed, bit for bit, + plus a distribution check. They are not upstream mps's draws (a different + generator), and this file does not claim they are. + +Nullifications this file is meant to catch: + +* `amax_keepdim_anywhere` back to `t.max_keepdim(dim)` on Metal + -> the all--inf and NaN rows of test_amax_agrees_on_mps +* the NaN half of it (`any_nan.where_cond`) dropped -> the NaN rows only +* `compare_common`'s device branch removed -> test_comparisons_agree_on_mps + (every float row raises again) +* `compare_scalar` not narrowing the scalar -> the f16 `== 0.1` row + (`host_const` refuses an f64 constant on Metal) +* `to_copy_default`'s device branch removed -> the `to(bool)` rows +* `normal_inplace` uploading f64 -> test_randn_and_normal_on_mps +""" + +import json +import math +import os +import subprocess +import sys + +os.environ.setdefault("TORCH_USE_RTLD_GLOBAL", "1") + +from test_shim import _C +import _skip + + +_REPO_ROOT = str(next(p for p in __import__("pathlib").Path(__file__).resolve().parents if (p / "AGENTS.md").is_file())) +_VENDOR_DIR = os.path.join(_REPO_ROOT, "torchnative", "python") + + +def _shim_tree_or_skip(what): + if not os.path.isfile(os.path.join(_VENDOR_DIR, "torch", "_C.abi3.so")): + _skip.skip(" (skipped %s: no vendored shim at %s -- run scripts/vendor/install_shim.sh)" + % (what, _VENDOR_DIR)) + return False + return True + + +def _mps_or_skip(what): + if not _shim_tree_or_skip(what): + return False + try: + _C._aten_dispatch("aten.ones.default", [1], device=_C.device("mps")) + except (NotImplementedError, RuntimeError) as e: + _skip.skip(" (skipped %s: no mps device on this machine -- %s)" + % (what, str(e).splitlines()[0])) + return False + return True + + +def _run(script, payload, upstream): + """Run `script` in a child. upstream=True empties PYTHONPATH so the child + can only import site-packages torch; otherwise the child imports the + vendored shim tree. Either way the script asserts which torch it got.""" + env = dict(os.environ) + env.pop("TORCH_C_ARTEFACT", None) + if upstream: + env.pop("PYTHONPATH", None) + env.pop("TORCH_USE_RTLD_GLOBAL", None) + else: + env["PYTHONPATH"] = _VENDOR_DIR + env["TORCH_USE_RTLD_GLOBAL"] = "1" + proc = subprocess.run([sys.executable, "-c", script, json.dumps(payload)], + capture_output=True, text=True, timeout=1800, env=env) + lines = proc.stdout.strip().splitlines() + if not lines: + raise AssertionError("child produced no JSON (rc=%d):\n%s" + % (proc.returncode, proc.stderr[-3000:])) + return json.loads(lines[-1]) + + +# One expression per child: a panic inside candle's Metal backend poisons the +# encoder's mutex for the rest of that process (test_mpsgen.py measured it), so +# one failure must not take the other rows with it. +_EXPR_SCRIPT = r""" +import json, sys +A = json.loads(sys.argv[1]) +import torch +IS_SHIM = hasattr(torch._C, "_aten_implemented") +assert IS_SHIM == A["shim"], ("wrong torch in this child", IS_SHIM) +D = A["device"] +inf, nan = float("inf"), float("nan") +DT = getattr(torch, A["dtype"]) +def T(rows): + return torch.tensor(rows, dtype=DT).to(D) +out = {} +try: + X = T(A["data"]) + before = torch._C._metal_counters() if IS_SHIM and D == "mps" else None + r = eval(A["expr"]) + after = torch._C._metal_counters() if before is not None else None + parts = list(r) if isinstance(r, tuple) else [r] + out = {"parts": [{"shape": list(p.shape), "dtype": str(p.dtype), + "device": str(p.device), + "values": p.cpu().double().flatten().tolist()} for p in parts]} + if before is not None: + out["counters"] = {k: after[k] - before[k] for k in before if k != "built"} +except BaseException as exc: + out = {"error": "%s: %s" % (type(exc).__name__, str(exc)[:300])} +print(json.dumps(out)) +""" + + +_INF, _NAN = float("inf"), float("nan") + +# (name, data, expr). `X` is the input on the device. +_EXTREMA_ROWS = [ + ("amax all -inf, dim=1", + [[-_INF, -_INF, -_INF], [1.0, -_INF, 2.0]], "torch.amax(X, dim=1)"), + ("amax all -inf, whole tensor", + [[-_INF, -_INF], [-_INF, -_INF]], "torch.amax(X)"), + ("amax all +inf, dim=0", + [[_INF, 1.0], [_INF, _INF]], "torch.amax(X, dim=0)"), + ("amax NaN not first", + [[1.0, _NAN, 3.0], [_NAN, 1.0, 2.0], [4.0, 5.0, _NAN]], "torch.amax(X, dim=1)"), + ("amax NaN, keepdim, dim=0", + [[1.0, 2.0], [_NAN, 0.5]], "torch.amax(X, dim=0, keepdim=True)"), + ("amax over two dims with -inf and NaN", + [[[-_INF, -_INF], [-_INF, -_INF]], [[1.0, _NAN], [2.0, 3.0]]], "torch.amax(X, dim=(1, 2))"), + ("max.default all -inf", [[-_INF, -_INF, -_INF]], "X.max()"), + ("min.default all +inf", [[_INF, _INF]], "X.min()"), + ("max.default NaN last", [[1.0, 2.0, _NAN]], "X.max()"), + ("min.default NaN middle", [[1.0, _NAN, -2.0]], "X.min()"), + ("max.dim all -inf and ties", + [[-_INF, -_INF, -_INF], [2.0, 5.0, 5.0], [-1.0, -1.0, -3.0]], "X.max(dim=1)"), + ("min.dim all +inf and ties", + [[_INF, _INF, _INF], [5.0, 2.0, 2.0], [-3.0, 0.0, -3.0]], "X.min(dim=1)"), + ("max.dim NaN first index", [[1.0, _NAN, _NAN, 4.0]], "X.max(dim=1)"), + ("min.dim NaN, keepdim, dim=0", + [[1.0, 2.0], [_NAN, 0.0], [_NAN, -1.0]], "X.min(dim=0, keepdim=True)"), +] + +_COMPARE_ROWS = [ + ("eq.Tensor", [[1.0, _NAN, -0.0, 3.0]], "X == T([[1.0, nan, 0.0, 4.0]])"), + ("ne.Tensor", [[1.0, _NAN, 2.0, 3.0]], "X != X"), + ("lt.Scalar", [[1.0, -_INF, 2.5, _NAN]], "X < 2.0"), + ("ge.Tensor broadcast", [[1.0, 4.0], [3.0, 2.0]], "X >= X[0:1]"), + ("eq.Scalar 0.1 is narrowed to the dtype", [[0.1, 0.2]], "X == 0.1"), + ("eq.Scalar int scalar", [[1.0, 2.0]], "X == 1"), + ("to(bool)", [[0.0, -0.0, _NAN, 2.0, -_INF]], "X.to(torch.bool)"), + ("eq on an empty tensor", [[1.0]], "X[:0] == X[:0]"), + ("eq.Scalar on an empty tensor", [[1.0]], "X[:0] == 1.0"), + ("to(bool) on an empty tensor", [[1.0]], "X[:0].to(torch.bool)"), +] + +_FLOAT_DTYPES = ["float32", "float16", "bfloat16"] + + +def _same(a, b): + if math.isnan(a) or math.isnan(b): + return math.isnan(a) and math.isnan(b) + return a == b + + +def _agree(rows): + problems = [] + for dtype in _FLOAT_DTYPES: + for name, data, expr in rows: + payload = dict(data=data, expr=expr, dtype=dtype) + want = _run(_EXPR_SCRIPT, dict(payload, shim=False, device="cpu"), upstream=True) + got = _run(_EXPR_SCRIPT, dict(payload, shim=True, device="mps"), upstream=False) + label = "%s [%s] %s" % (name, dtype, expr) + assert "error" not in want, (label, want) + if "error" in got: + problems.append("%s: shim raised %s" % (label, got["error"])) + continue + if len(got["parts"]) != len(want["parts"]): + problems.append("%s: %d results, upstream %d" + % (label, len(got["parts"]), len(want["parts"]))) + continue + for i, (g, w) in enumerate(zip(got["parts"], want["parts"])): + if not g["device"].startswith("mps"): + problems.append("%s: result %d on %s" % (label, i, g["device"])) + if (g["shape"], g["dtype"]) != (w["shape"], w["dtype"]): + problems.append("%s: result %d is %s %s, upstream %s %s" + % (label, i, g["shape"], g["dtype"], w["shape"], w["dtype"])) + elif not all(_same(a, b) for a, b in zip(g["values"], w["values"])): + problems.append("%s: result %d is %s, upstream %s" + % (label, i, g["values"], w["values"])) + c = got["counters"] + empty = all(len(p["values"]) == 0 for p in got["parts"]) + if not empty and c["compute_encoders"] <= 0: + problems.append("%s: no compute encoder ran (%s)" % (label, c)) + if c["host_downloads"] != 0: + problems.append("%s: the operand was read back to the host (%s)" % (label, c)) + assert not problems, "\n".join(problems) + + +def test_amax_agrees_on_mps(): + """amax / max / min (with and without dim) on rows of all -inf, all +inf, + NaN in every position and ties, in float32/float16/bfloat16: values and + indices equal upstream's exactly (indices: the first extremum, or the + first NaN), and each dispatch launched a compute encoder and downloaded + nothing.""" + if not _mps_or_skip("extrema on mps"): + return + _agree(_EXTREMA_ROWS) + + +def test_comparisons_agree_on_mps(): + """Float eq/ne/lt/ge (Tensor and Scalar overloads) and to(torch.bool), on + non-empty and empty mps tensors: the booleans equal upstream's, the result + stays on mps, and nothing is read back.""" + if not _mps_or_skip("comparisons on mps"): + return + _agree(_COMPARE_ROWS) + + +_RANDN_SCRIPT = r""" +import json, sys +A = json.loads(sys.argv[1]) +import torch +assert hasattr(torch._C, "_aten_implemented"), "this child must import the shim" +out = {} +try: + DT = getattr(torch, A["dtype"]) + draws = {} + for D in ("cpu", "mps"): + torch.manual_seed(A["seed"]) + before = torch._C._metal_counters() if D == "mps" else None + if A["how"] == "randn": + r = torch.randn(A["n"], dtype=DT, device=D) + else: + r = torch.empty(A["n"], dtype=DT, device=D).normal_(2.0, 0.5) + after = torch._C._metal_counters() if D == "mps" else None + draws[D] = {"device": str(r.device), "values": r.cpu().double().tolist()} + if before is not None: + draws[D]["counters"] = {k: after[k] - before[k] for k in before if k != "built"} + out = draws +except BaseException as exc: + out = {"error": "%s: %s" % (type(exc).__name__, str(exc)[:300])} +print(json.dumps(out)) +""" + + +def test_randn_and_normal_on_mps(): + """torch.randn and Tensor.normal_ on mps, at sizes on both sides of + normal_'s 16-element path split: they succeed, the result is on mps, the + draws equal this shim's CPU draws from the same seed bit for bit (one + stream, narrowed on the host), the bracket uploaded and downloaded + nothing, and the 4096-draw sample has the asked-for mean and spread + (|mean - mu| < 5 sigma/sqrt(n), std within 10%).""" + if not _mps_or_skip("randn on mps"): + return + problems = [] + for dtype in _FLOAT_DTYPES: + for how in ("randn", "normal_"): + for n in (15, 17, 4096): + label = "%s(%d) %s" % (how, n, dtype) + got = _run(_RANDN_SCRIPT, dict(dtype=dtype, how=how, n=n, seed=7), + upstream=False) + if "error" in got: + problems.append("%s: %s" % (label, got["error"])) + continue + mps, cpu = got["mps"], got["cpu"] + if not mps["device"].startswith("mps"): + problems.append("%s: result on %s" % (label, mps["device"])) + if mps["values"] != cpu["values"]: + problems.append("%s: mps draws differ from the CPU stream" % label) + c = mps["counters"] + if c["host_uploads"] <= 0 or c["host_downloads"] != 0: + problems.append("%s: expected an upload and no download, got %s" % (label, c)) + if n == 4096: + mu, sigma = (0.0, 1.0) if how == "randn" else (2.0, 0.5) + v = mps["values"] + mean = sum(v) / n + std = math.sqrt(sum((x - mean) ** 2 for x in v) / (n - 1)) + if abs(mean - mu) > 5 * sigma / math.sqrt(n) or abs(std / sigma - 1) > 0.1: + problems.append("%s: mean %.4f std %.4f, asked for %s, %s" + % (label, mean, std, mu, sigma)) + assert not problems, "\n".join(problems) + + +def _main(): + items = [(name, fn) for name, fn in sorted(globals().items()) + if name.startswith("test_")] + failures = _skip.run_tests(items, suite="test_mpsextreme") + return 1 if failures else 0 + + +if __name__ == "__main__": + raise SystemExit(_main()) diff --git a/tests/devices/test_dtypedev.py b/tests/devices/test_dtypedev.py index 7ecb4def..36061ba1 100644 --- a/tests/devices/test_dtypedev.py +++ b/tests/devices/test_dtypedev.py @@ -870,6 +870,7 @@ def test_every_dtype_this_build_cannot_store_refuses_by_name(): "bfloat16|add", "bfloat16|clone", "bfloat16|div", + "bfloat16|eq", "bfloat16|exp", "bfloat16|index0", "bfloat16|matmul", @@ -896,6 +897,7 @@ def test_every_dtype_this_build_cannot_store_refuses_by_name(): "float16|add", "float16|clone", "float16|div", + "float16|eq", "float16|exp", "float16|index0", "float16|matmul", @@ -912,6 +914,7 @@ def test_every_dtype_this_build_cannot_store_refuses_by_name(): "float32|add", "float32|clone", "float32|div", + "float32|eq", "float32|exp", "float32|index0", "float32|matmul", diff --git a/tests/golden/cases.py b/tests/golden/cases.py index 4eb93cc7..ee4e3b59 100644 --- a/tests/golden/cases.py +++ b/tests/golden/cases.py @@ -44,7 +44,7 @@ import math import platform -from dataclasses import dataclass +from dataclasses import dataclass, replace from typing import Any, Callable import dtypes as dt @@ -68,6 +68,11 @@ class Case: # matched seed (randint). Signature: (torch_result, c_result) -> # (ok, detail_message). value_check: Callable[[Any, Any], tuple[bool, str]] | None = None + # The default pipeline compares the sign of an exact zero only when this + # is set (compare.py `_values_close`). Set it where upstream promises the + # sign -- `min.default`/`max.default` order `-0 < +0` (issue #49) -- and + # nowhere it is a vectorisation artefact (`amax`). + signed_zero: bool = False # --- aten.full.default ------------------------------------------------- @@ -7916,6 +7921,37 @@ def max_default_cases(torch_module, c_module, torch_call) -> list[Case]: f"NaN is NaN (measured) -- torch's rule is IEEE maximum, not fmax", ) ) + cases += _signed_zero_extremum_cases(torch_module, c_module, op, torch_call, "max") + return cases + + +def _signed_zero_extremum_cases(torch_module, c_module, op, torch_call, short) -> list[Case]: + """`min()`/`max()` of a tensor whose only extremum is a zero of both signs. + + Upstream's whole-tensor reduction orders `-0.0 < +0.0`: `min` answers + `-0.0` when any `-0.0` is present and `max` answers `+0.0` when any `+0.0` + is, wherever it sits (measured on torch 2.13.0 for every position of + lengths 1..69 in float32, float16 and bfloat16). The `dim=` overloads do + not: they answer the element at the first equal index. This build answered + the first index for both, so `min([0., -0., 0., -0.])` was `0.0` (issue + #49), invisible until the comparator compared the sign. + """ + cases: list[Case] = [] + layouts = [ + ([0.0, -0.0, 0.0, -0.0], (4,), "first element +0"), + ([-0.0, 0.0, -0.0, 0.0], (4,), "first element -0"), + ([0.0, 0.0, 0.0, -0.0], (2, 2), "the other sign only last"), + ([-0.0, -0.0, -0.0, 0.0], (2, 2), "the other sign only last, mirrored"), + ([-0.0, -0.0], (2,), "every zero negative"), + ([0.0, 0.0], (2,), "every zero positive"), + ] + for flat, shape, where in layouts: + for dtype_name in ["float64", "float32", "float16", "bfloat16"]: + case = _unary_case( + torch_module, c_module, op, torch_call, dtype_name, flat, shape, + f"signed zero, {where}: {short}() orders -0 < +0 (measured)", + ) + cases.append(replace(case, signed_zero=True)) return cases @@ -21813,6 +21849,7 @@ def min_default_cases(torch_module, c_module, torch_call) -> list[Case]: f"NaN is NaN (measured)", ) ) + cases += _signed_zero_extremum_cases(torch_module, c_module, op, torch_call, "min") # No empty-tensor case: `max_default_cases` does not have one either -- # measured, torch's `min()` raises on an empty input ("Expected reduction # dim to be specified for input.numel() == 0"), so it is out of scope for @@ -31367,3 +31404,86 @@ def wrapped(torch_module, c_module, torch_call): assert _op in CASE_BUILDERS, _op CASE_BUILDERS[_op] = _with_issue29(_op, CASE_BUILDERS[_op]) del _op + + +# --- issue #93: a Python float scalar is narrowed to the comparison dtype -------- +# +# Upstream casts a Python scalar to the comparison dtype (the tensor's float +# dtype, or the default float for an integral tensor) before comparing, so +# `float32([0.1]) == 0.1` is True and `float16([inf]) == 65520.0` is True +# (65520 rounds to inf in float16). The cpu path widened both sides to f64 +# and answered False. Measured on upstream 2.13.0 (cpu). Every value below is +# one where the narrowing decides the answer: comparing the unnarrowed scalar +# flips at least one element. +_CMP93_ROWS = [ + # (dtype, tensor values, scalar) + ("float32", [0.1, 0.2], 0.1), + ("float32", [1 / 3, 0.0], 1 / 3), + ("float32", [0.30000001192092896, 0.3], 0.3), + ("float64", [0.30000001192092896, 0.3], 0.3), + ("float16", [0.1, 0.0], 0.1), + ("float16", [1 / 3, 1.0], 1 / 3), + # Between 1+2**-10 and 1+2**-9 in float16, closer to the first. + ("float16", [1.0009765625, 1.001953125], 1.0009765625 + 2 ** -12), + ("float16", [float("inf"), 65504.0], 65520.0), + ("bfloat16", [0.1, 0.0], 0.1), + ("bfloat16", [1 / 3, 1.0], 1 / 3), + # Between 1+2**-7 and 1+2**-6 in bfloat16, closer to the first. + ("bfloat16", [1.0078125, 1.015625], 1.0078125 + 2 ** -9), + # Halfway between bfloat16's max and 2**128: rounds (to even) to inf. + ("bfloat16", [float("inf"), 3.3895313892515355e38], 2.0 ** 128 * (1 - 2 ** -9)), + # Integral tensor meeting a float scalar: compared in the default float. + ("int64", [16777217, 16777215], 16777216.0), + ("int32", [16777217, 3], 16777216.0), + # NaN and inf scalars. + ("float16", [float("nan"), 1.0], float("nan")), + ("float32", [float("inf"), 1.0], float("inf")), +] + + +def _issue93_extra(op, torch_module, c_module, torch_call): + out: list[Case] = [] + if op.endswith(".Scalar"): + for dtype_name, values, scalar in _CMP93_ROWS: + out.append( + _binary_scalar_case( + torch_module, c_module, op, torch_call, dtype_name, values, (len(values),), + scalar, "issue #93: the scalar is narrowed to the comparison dtype", + ) + ) + return out + # `.Tensor` with a 0-dim float64 operand: a 0-dim tensor does not promote a + # dimensioned float tensor, so it is narrowed to float32 (True on eq); it + # does promote an integral one, to its own float64 (False on eq). + for dtype_name, values, scalar in (("float32", [0.1, 0.2], 0.1), ("float16", [0.1, 0.0], 0.1), + ("int64", [16777217, 16777215], 16777216.0)): + a_t, a_c = pair_from_flat(torch_module, c_module, values, (len(values),), dtype_name) + b_t, b_c = pair_from_flat(torch_module, c_module, [scalar], (), "float64") + out.append(Case( + name=f"{op}({dtype_name} vs 0-dim float64 {scalar!r}) [issue #93]", + op=op, + run_torch=lambda a=a_t, b=b_t: torch_call(a, b), + run_c=lambda a=a_c, b=b_c: c_module._aten_dispatch(op, a, b), + note="issue #93: a 0-dim float64 tensor follows dimensioned-category promotion", + )) + return out + + +def _with_issue93(op, builder): + def wrapped(torch_module, c_module, torch_call): + return list(builder(torch_module, c_module, torch_call)) + _issue93_extra( + op, torch_module, c_module, torch_call + ) + + return wrapped + + +for _op in ( + "aten.eq.Scalar", "aten.ne.Scalar", "aten.lt.Scalar", "aten.le.Scalar", + "aten.gt.Scalar", "aten.ge.Scalar", + "aten.eq.Tensor", "aten.ne.Tensor", "aten.lt.Tensor", "aten.le.Tensor", + "aten.gt.Tensor", "aten.ge.Tensor", +): + assert _op in CASE_BUILDERS, _op + CASE_BUILDERS[_op] = _with_issue93(_op, CASE_BUILDERS[_op]) +del _op diff --git a/tests/golden/compare.py b/tests/golden/compare.py index cc3240a8..0c9d209c 100644 --- a/tests/golden/compare.py +++ b/tests/golden/compare.py @@ -98,7 +98,29 @@ class Outcome: _NLL = ("#69 (#40 class A5: vectorised accumulation)", "weighted nll_loss sum/mean: x86 upstream's vectorised accumulation rounds the weighted terms differently") _GELU = ("#69", "gelu erf form, float32 and bfloat16: x86 upstream returns NaN for gelu(+inf) where arm64 upstream and the shim return +inf") +_SZERO = ("#49 (#40)", "whole-tensor min/max zero sign: x86_64 upstream's vectorised reduction answers the other zero than arm64 upstream and the shim") + KNOWN_X86_64_LINUX: dict[str, list[tuple[str, str, str]]] = { + 'aten.max.default': [ + ('max(dtype=bfloat16, shape=(2, 2)) [signed zero, the other sign only last: max() orders -0 < +0 (measured)]', *_SZERO), + ('max(dtype=bfloat16, shape=(4,)) [signed zero, first element +0: max() orders -0 < +0 (measured)]', *_SZERO), + ('max(dtype=float16, shape=(2, 2)) [signed zero, the other sign only last: max() orders -0 < +0 (measured)]', *_SZERO), + ('max(dtype=float16, shape=(4,)) [signed zero, first element +0: max() orders -0 < +0 (measured)]', *_SZERO), + ('max(dtype=float32, shape=(2, 2)) [signed zero, the other sign only last: max() orders -0 < +0 (measured)]', *_SZERO), + ('max(dtype=float32, shape=(4,)) [signed zero, first element +0: max() orders -0 < +0 (measured)]', *_SZERO), + ('max(dtype=float64, shape=(2, 2)) [signed zero, the other sign only last: max() orders -0 < +0 (measured)]', *_SZERO), + ('max(dtype=float64, shape=(4,)) [signed zero, first element +0: max() orders -0 < +0 (measured)]', *_SZERO), + ], + 'aten.min.default': [ + ('min(dtype=bfloat16, shape=(2, 2)) [signed zero, the other sign only last, mirrored: min() orders -0 < +0 (measured)]', *_SZERO), + ('min(dtype=bfloat16, shape=(4,)) [signed zero, first element -0: min() orders -0 < +0 (measured)]', *_SZERO), + ('min(dtype=float16, shape=(2, 2)) [signed zero, the other sign only last, mirrored: min() orders -0 < +0 (measured)]', *_SZERO), + ('min(dtype=float16, shape=(4,)) [signed zero, first element -0: min() orders -0 < +0 (measured)]', *_SZERO), + ('min(dtype=float32, shape=(2, 2)) [signed zero, the other sign only last, mirrored: min() orders -0 < +0 (measured)]', *_SZERO), + ('min(dtype=float32, shape=(4,)) [signed zero, first element -0: min() orders -0 < +0 (measured)]', *_SZERO), + ('min(dtype=float64, shape=(2, 2)) [signed zero, the other sign only last, mirrored: min() orders -0 < +0 (measured)]', *_SZERO), + ('min(dtype=float64, shape=(4,)) [signed zero, first element -0: min() orders -0 < +0 (measured)]', *_SZERO), + ], 'aten._scaled_dot_product_flash_attention_for_cpu.default': [ ('sdpa_flash_cpu(dtype=bfloat16, q_len=33, kv_len=33, mask=False, is_causal=True) [second query block, causal]', *_A3), ('sdpa_flash_cpu(dtype=float16, q_len=33, kv_len=33, mask=False, is_causal=True) [second query block, causal]', *_A3), @@ -212,7 +234,20 @@ def _flatten(x) -> list: return [x] -def _values_close(torch_nested, c_nested, atol: float, rtol: float) -> tuple[bool, str]: +def _values_close( + torch_nested, c_nested, atol: float, rtol: float, signed_zero: bool = False +) -> tuple[bool, str]: + """Element-wise agreement within tolerance. + + `signed_zero` makes `+0.0` and `-0.0` two different answers. A tolerance + cannot see the difference (`isclose(0.0, -0.0)` is True), so without it + `min([0., -0.])` answering `0.0` where upstream answers `-0.0` passed + (issue #49). It is per case rather than global because upstream promises + the sign only for some ops: `min.default`/`max.default` order `-0 < +0` + at every length measured, while `amax`'s sign of a zero tie moves with the + vectorised lane count (n=4 keeps the first element, n=17 does not; + measured on torch 2.13.0), so pinning it there would pin an artefact. + """ ta, ca = _flatten(torch_nested), _flatten(c_nested) if len(ta) != len(ca): return False, f"flattened length differs: torch has {len(ta)}, c has {len(ca)}" @@ -239,6 +274,10 @@ def _values_close(torch_nested, c_nested, atol: float, rtol: float) -> tuple[boo if xf != yf: return False, f"index {i}: torch={x!r} c={y!r} (inf mismatch)" continue + if signed_zero and xf == 0.0 and yf == 0.0: + if math.copysign(1.0, xf) != math.copysign(1.0, yf): + return False, f"index {i}: torch={x!r} c={y!r} (sign of zero differs)" + continue if not math.isclose(xf, yf, rel_tol=rtol, abs_tol=atol): return ( False, @@ -328,7 +367,19 @@ def _run_one_body(case: Case, inject_fault: str | None, tag_box: list[str]) -> O c_exc = e fault_tag = "" - if inject_fault and t_res is not None and c_res is not None: + if inject_fault == "zero-sign": + # Only a case that promises the sign of zero can be asked to see it + # flipped; for every other case the mode is not constructible, which + # the self-test reports as n/a rather than as a miss. + if ( + t_res is not None + and c_res is not None + and getattr(case, "signed_zero", False) + and case.value_check is None + ): + c_res, fault_tag = _corrupt_zero_sign(c_res) + tag_box[0] = fault_tag + elif inject_fault and t_res is not None and c_res is not None: c_res, fault_tag = _corrupt(c_res, inject_fault) tag_box[0] = fault_tag @@ -446,7 +497,10 @@ def _run_one_body(case: Case, inject_fault: str | None, tag_box: list[str]) -> O return Outcome(case, False, f"{prefix}shape mismatch: torch={t_shape} c={c_shape}") tol = dt.tolerance_for(t_dtype) - ok, detail = _values_close(_as_list(t_res), _as_list(c_res), tol.atol, tol.rtol) + ok, detail = _values_close( + _as_list(t_res), _as_list(c_res), tol.atol, tol.rtol, + signed_zero=getattr(case, "signed_zero", False), + ) if not ok: return Outcome( case, @@ -507,6 +561,10 @@ def _run_one_body(case: Case, inject_fault: str | None, tag_box: list[str]) -> O # constant every element collapsed to the first -- the shape a # broken RNG takes, and what a pure range check misses # chunk-count a chunk dropped +# zero-sign the first exact zero of the result negated +# (`0.0` <-> `-0.0`), only for a case with +# `signed_zero=True` (issue #49). Every tolerance +# accepts it, so only the sign comparison can see it # chunk-pad the last chunk PADDED to full width instead of left # short. docs/models/GPT2.md names this as the most plausible # misimplementation of `split`, and it is invisible to @@ -527,6 +585,7 @@ def _run_one_body(case: Case, inject_fault: str | None, tag_box: list[str]) -> O "constant", "chunk-count", "chunk-pad", + "zero-sign", ) # (comparator, mode) pairs where NOT catching the fault is the documented, @@ -794,6 +853,28 @@ def _corrupt(result, mode: str) -> tuple[Any, str]: return _recompose(kind, members), f"INJECTED {mode} fault" +def _corrupt_zero_sign(result) -> tuple[Any, str]: + """The first exact float zero of a one-tensor result with its sign + flipped. `(result, "")` when the result holds no float zero.""" + kind, members = _decompose(result) + if kind == "scalar" or len(members) != 1: + return result, "" + target = members[0] + values = _as_list(target) + flat = _flatten(values) + hit = next( + (i for i, v in enumerate(flat) if isinstance(v, float) and v == 0.0), + None, + ) + if hit is None: + return result, "" + flat = list(flat) + flat[hit] = -flat[hit] + shape = tuple(int(v) for v in target.shape) + fake = _FakeResult(_reshape(flat, shape), target.dtype, shape) + return _recompose(kind, [fake]), "INJECTED zero-sign fault" + + def _uncaught_suffix(case: Case, fault_tag: str, mode: str | None) -> str: """What to say when an injected fault sailed through. Only "COMPARATOR BUG" if the comparator was supposed to catch it.""" @@ -1021,9 +1102,23 @@ def self_test(artefact: str | None, verbose: bool, scan_limit: int) -> int: for cmp_name in sorted(groups): group = groups[cmp_name] pending = set(FAULT_MODES) - for case in group[:scan_limit]: + # The signed-zero cases are few and sit deep in the default group, so + # they are appended past the scan limit: otherwise `zero-sign` would + # read n/a for the one comparator that is meant to catch it. + candidates = group[:scan_limit] + [ + c for c in group[scan_limit:] if getattr(c, "signed_zero", False) + ] + for case in candidates: if not pending: break + # A case must pass clean before a corruption of it means anything. + # On x86_64 the shim's zero sign for max() differs from upstream's + # (issue #49, PR #83 CI), so that case fails clean, and flipping + # the shim's zero lands on upstream's answer: read as a MISS + # though the comparator is right. A failing case is reported by + # the golden run itself, not here. + if not _run_one(case, None).passed: + continue for mode in sorted(pending): outcome = _run_one(case, mode) if not outcome.fault_applied: diff --git a/tests/golden/test_x86pins.py b/tests/golden/test_x86pins.py index 78611361..dc678542 100644 --- a/tests/golden/test_x86pins.py +++ b/tests/golden/test_x86pins.py @@ -87,7 +87,7 @@ def test_every_pin_names_a_case_cases_py_builds(): if name not in names[op]: dead.append((op, name)) assert not dead, f"{len(dead)} pin(s) name no case cases.py builds: {dead}" - assert total == 54, f"expected the 54 pinned names of the PR #90 CI run, found {total}" + assert total == 70, f"expected the 70 pinned names (54 from PR #90's CI run, 16 signed-zero from PR #94's), found {total}" def test_no_pin_is_a_whole_op_or_a_duplicate(): diff --git a/tests/ops/test_cmpscalar.py b/tests/ops/test_cmpscalar.py new file mode 100644 index 00000000..cb921de5 --- /dev/null +++ b/tests/ops/test_cmpscalar.py @@ -0,0 +1,250 @@ +"""A Python float scalar is narrowed to the comparison dtype (issue #93). + +Upstream casts a Python scalar to the comparison dtype before comparing: the +tensor's own float dtype, or the default float (float32) for an integral +tensor. So `float32([0.1]) == 0.1` is True, `float16([inf]) == 65520.0` is +True (65520 rounds to inf in float16), and `int64([16777217]) == 16777216.0` +is True (both become float32 16777216). Measured on torch 2.13.0 +(`.caches/spike-venv`, cpu). The cpu path here widened both sides to f64 and +answered False for every one of them; mps already narrowed (PR #83). + +A 0-dim float64 *tensor* (not a wrapped Python number) follows ordinary +promotion: it does not promote a dimensioned float tensor (narrowed, so +`float32([0.1]) == tensor(0.1, float64)` is True) but it does promote an +integral one, to float64 (so `int64([16777217]) == tensor(16777216.0, +float64)` is False). + +Nullification: in `compare_scalar` (aten.rs), compare the unnarrowed +`other.as_f64()` instead of `float_narrower(ctag)(...)` and +`test_scalar_rows_match_upstream` goes red on every float16/bfloat16/float32 +row; drop the default-float cast of an integral operand in `compare_common` +and the int64 row goes red. +""" + +import os + +os.environ.setdefault("TORCH_USE_RTLD_GLOBAL", "1") + +import _C +import _skip + +try: + import torch as _upstream_torch +except ImportError: # pragma: no cover - most interpreters running this file + _upstream_torch = None + + +_DTYPES = {"float32": _C.float32, "float64": _C.float64, "float16": _C.float16, + "bfloat16": _C.bfloat16, "int64": _C.int64} + +_INF = float("inf") +_NAN = float("nan") + +# (dtype, tensor values, scalar, eq, lt, gt): upstream 2.13.0 cpu answers. +# ne = not eq, le = eq or lt, ge = eq or gt (holds for NaN too). +_ROWS = [ + ("float32", [0.1, 0.2], 0.1, [True, False], [False, False], [False, True]), + ("float32", [1 / 3, 0.0], 1 / 3, [True, False], [False, True], [False, False]), + ("float64", [0.30000001192092896, 0.3], 0.3, [False, True], [False, False], [True, False]), + ("float16", [0.1, 0.0], 0.1, [True, False], [False, True], [False, False]), + ("float16", [1.0009765625, 1.001953125], 1.0009765625 + 2 ** -12, + [True, False], [False, False], [False, True]), + ("float16", [_INF, 65504.0], 65520.0, [True, False], [False, True], [False, False]), + ("bfloat16", [0.1, 0.0], 0.1, [True, False], [False, True], [False, False]), + ("bfloat16", [1.0078125, 1.015625], 1.0078125 + 2 ** -9, + [True, False], [False, False], [False, True]), + ("bfloat16", [_INF, 3.3895313892515355e38], 2.0 ** 128 * (1 - 2 ** -9), + [True, False], [False, True], [False, False]), + ("int64", [16777217, 16777215], 16777216.0, [True, False], [False, True], [False, False]), + ("float16", [_NAN, 1.0], _NAN, [False, False], [False, False], [False, False]), + ("float32", [_INF, 1.0], _INF, [True, False], [False, True], [False, False]), +] + +# (dtype, values, 0-dim float64 value, eq, lt, gt) for the `.Tensor` overloads. +_ZERO_DIM = [ + ("float32", [0.1, 0.2], 0.1, [True, False], [False, False], [False, True]), + ("float16", [0.1, 0.0], 0.1, [True, False], [False, True], [False, False]), + ("int64", [16777217, 16777215], 16777216.0, [False, False], [False, True], [True, False]), +] + + +def _expected(eq, lt, gt): + return { + "eq": eq, "lt": lt, "gt": gt, + "ne": [not e for e in eq], + "le": [e or l for e, l in zip(eq, lt)], + "ge": [e or g for e, g in zip(eq, gt)], + } + + +def _shim(values, dtype, shape=None): + return _C._tensor_from_flat(values, [len(values)] if shape is None else shape, + _DTYPES[dtype]) + + +def test_scalar_rows_match_upstream(): + problems = [] + for dtype, values, scalar, eq, lt, gt in _ROWS: + for name, want in _expected(eq, lt, gt).items(): + got = _C._aten_dispatch("aten.%s.Scalar" % name, _shim(values, dtype), scalar).tolist() + if got != want: + problems.append("%s.Scalar(%s %s, %r) = %r, upstream %r" + % (name, dtype, values, scalar, got, want)) + assert not problems, "\n".join(problems) + + +def test_zero_dim_float64_tensor_operand(): + problems = [] + for dtype, values, other, eq, lt, gt in _ZERO_DIM: + for name, want in _expected(eq, lt, gt).items(): + got = _C._aten_dispatch("aten.%s.Tensor" % name, _shim(values, dtype), + _shim([other], "float64", [])).tolist() + if got != want: + problems.append("%s.Tensor(%s %s, 0-dim float64 %r) = %r, upstream %r" + % (name, dtype, values, other, got, want)) + assert not problems, "\n".join(problems) + + +def test_the_pinned_answers_are_upstreams(): + """Re-asks upstream in this process, so the tables above are not values + written down once. Skipped by name when upstream is not importable, or + when `torch` here is the shim.""" + if _upstream_torch is None or hasattr(_upstream_torch._C, "_aten_implemented"): + _skip.skip(" (skipped test_the_pinned_answers_are_upstreams: no upstream torch " + "in this interpreter)") + return + T = _upstream_torch + problems = [] + for rows, zero_dim in ((_ROWS, False), (_ZERO_DIM, True)): + for dtype, values, other, eq, lt, gt in rows: + t = T.tensor(values, dtype=getattr(T, dtype)) + o = T.tensor(other, dtype=T.float64) if zero_dim else other + for name, want in _expected(eq, lt, gt).items(): + got = getattr(T, name)(t, o).tolist() + if got != want: + problems.append("upstream %s(%s %s, %r%s) = %r, table says %r" + % (name, dtype, values, other, + " 0-dim" if zero_dim else "", got, want)) + assert not problems, "\n".join(problems) + + +# add(dimensioned, 0-dim) result dtypes, upstream 2.13.0 cpu (bool+bool via +# `|`, since add refuses it). c10's ResultTypeState: within a category the +# dimensioned operand wins; a higher category on the 0-dim side still +# promotes. issue #93 found the shim promoting float32[2] + tensor(0.1, +# float64) to float64. Complex rows are left out: `aten.ones` cannot build a +# complex tensor in this shim, so combine_categories' complex branch is not +# exercised here. +_PROMOTION = [ + ('bool', 'bool', 'bool'), + ('bool', 'uint8', 'uint8'), + ('bool', 'int32', 'int32'), + ('bool', 'int64', 'int64'), + ('bool', 'float16', 'float16'), + ('bool', 'bfloat16', 'bfloat16'), + ('bool', 'float32', 'float32'), + ('bool', 'float64', 'float64'), + ('uint8', 'bool', 'uint8'), + ('uint8', 'uint8', 'uint8'), + ('uint8', 'int32', 'uint8'), + ('uint8', 'int64', 'uint8'), + ('uint8', 'float16', 'float16'), + ('uint8', 'bfloat16', 'bfloat16'), + ('uint8', 'float32', 'float32'), + ('uint8', 'float64', 'float64'), + ('int32', 'bool', 'int32'), + ('int32', 'uint8', 'int32'), + ('int32', 'int32', 'int32'), + ('int32', 'int64', 'int32'), + ('int32', 'float16', 'float16'), + ('int32', 'bfloat16', 'bfloat16'), + ('int32', 'float32', 'float32'), + ('int32', 'float64', 'float64'), + ('int64', 'bool', 'int64'), + ('int64', 'uint8', 'int64'), + ('int64', 'int32', 'int64'), + ('int64', 'int64', 'int64'), + ('int64', 'float16', 'float16'), + ('int64', 'bfloat16', 'bfloat16'), + ('int64', 'float32', 'float32'), + ('int64', 'float64', 'float64'), + ('float16', 'bool', 'float16'), + ('float16', 'uint8', 'float16'), + ('float16', 'int32', 'float16'), + ('float16', 'int64', 'float16'), + ('float16', 'float16', 'float16'), + ('float16', 'bfloat16', 'float16'), + ('float16', 'float32', 'float16'), + ('float16', 'float64', 'float16'), + ('bfloat16', 'bool', 'bfloat16'), + ('bfloat16', 'uint8', 'bfloat16'), + ('bfloat16', 'int32', 'bfloat16'), + ('bfloat16', 'int64', 'bfloat16'), + ('bfloat16', 'float16', 'bfloat16'), + ('bfloat16', 'bfloat16', 'bfloat16'), + ('bfloat16', 'float32', 'bfloat16'), + ('bfloat16', 'float64', 'bfloat16'), + ('float32', 'bool', 'float32'), + ('float32', 'uint8', 'float32'), + ('float32', 'int32', 'float32'), + ('float32', 'int64', 'float32'), + ('float32', 'float16', 'float32'), + ('float32', 'bfloat16', 'float32'), + ('float32', 'float32', 'float32'), + ('float32', 'float64', 'float32'), + ('float64', 'bool', 'float64'), + ('float64', 'uint8', 'float64'), + ('float64', 'int32', 'float64'), + ('float64', 'int64', 'float64'), + ('float64', 'float16', 'float64'), + ('float64', 'bfloat16', 'float64'), + ('float64', 'float32', 'float64'), + ('float64', 'float64', 'float64'), +] + + +def _shim_result_dtype(a, b): + t = _C._aten_dispatch("aten.ones.default", [2], dtype=getattr(_C, a)) + z = _C._aten_dispatch("aten.ones.default", [], dtype=getattr(_C, b)) + op = "aten.bitwise_or.Tensor" if a == b == "bool" else "aten.add.Tensor" + return str(_C._aten_dispatch(op, t, z).dtype).split(".")[-1] + + +def test_a_zero_dim_operand_does_not_outrank_a_dimensioned_one(): + problems = [] + for a, b, want in _PROMOTION: + try: + got = _shim_result_dtype(a, b) + except Exception as e: # noqa: BLE001 + got = "%s: %s" % (type(e).__name__, str(e)[:80]) + if got != want: + problems.append("add(%s[2], %s 0-dim) -> %s, upstream %s" % (a, b, got, want)) + assert not problems, "%d of %d differ:\n%s" % (len(problems), len(_PROMOTION), "\n".join(problems)) + + +def test_the_promotion_table_is_upstreams(): + if _upstream_torch is None or hasattr(_upstream_torch._C, "_aten_implemented"): + _skip.skip(" (skipped test_the_promotion_table_is_upstreams: no upstream torch " + "in this interpreter)") + return + T = _upstream_torch + problems = [] + for a, b, want in _PROMOTION: + t = T.ones(2, dtype=getattr(T, a)) + z = T.tensor(1, dtype=getattr(T, b)) + got = \ + str(((t | z) if a == b == "bool" else (t + z)).dtype).split(".")[-1] + if got != want: + problems.append("upstream add(%s[2], %s 0-dim) = %s, table says %s" % (a, b, got, want)) + assert not problems, "\n".join(problems) + + +def _main(): + items = [(name, fn) for name, fn in sorted(globals().items()) + if name.startswith("test_")] + failures = _skip.run_tests(items, suite="test_cmpscalar") + return 1 if failures else 0 + + +if __name__ == "__main__": + raise SystemExit(_main()) diff --git a/tests/ops/test_signedzero.py b/tests/ops/test_signedzero.py new file mode 100644 index 00000000..4c525634 --- /dev/null +++ b/tests/ops/test_signedzero.py @@ -0,0 +1,118 @@ +"""The sign of a zero answer from `min()` / `max()` (issue #49). + +Upstream's whole-tensor `min()`/`max()` order `-0.0 < +0.0`: `min` answers +`-0.0` whenever a `-0.0` is present and `max` answers `+0.0` whenever a `+0.0` +is, wherever it sits. Measured on torch 2.13.0 +(`.caches/spike-venv`) for every position of lengths 1..69 in float32, float16 +and bfloat16, zero exceptions. The `dim=` overloads follow a different rule +(the first equal element), so `min([0., -0.], dim=0)` is `0.0` upstream and +that is pinned here too, so a fix cannot overreach into them. + +This build answered the first equal element for both, so `min([0., -0., 0., +-0.])` was `0.0`. The golden comparator could not see it (`isclose(0.0, -0.0)` +is True); compare.py now compares the sign for cases marked `signed_zero`, and +`min_default_cases`/`max_default_cases` carry them. This file pins the same +answers without the harness, and re-asks upstream live when the interpreter +has it. + +Nullification: `extremum_default` without its `signed_zero_extremum` call +turns `test_min_max_default_order_negative_zero_first` red (the `min` rows +answer `0.0`, the `max` rows `-0.0`). +""" + +import math +import os + +os.environ.setdefault("TORCH_USE_RTLD_GLOBAL", "1") + +import _C +import _skip + +try: + import torch as _upstream_torch +except ImportError: # pragma: no cover - most interpreters running this file + _upstream_torch = None + + +_DTYPES = {"float32": _C.float32, "float64": _C.float64, + "float16": _C.float16, "bfloat16": _C.bfloat16} + +# (values, op, expected sign of the zero answer) -- upstream's answers, +# measured as stated in the module docstring. +_CASES = [ + ([0.0, -0.0, 0.0, -0.0], "aten.min.default", -1.0), + ([-0.0, 0.0, -0.0, 0.0], "aten.min.default", -1.0), + ([0.0, 0.0, 0.0, -0.0], "aten.min.default", -1.0), + ([0.0, 0.0], "aten.min.default", 1.0), + ([0.0, -0.0, 0.0, -0.0], "aten.max.default", 1.0), + ([-0.0, -0.0, -0.0, 0.0], "aten.max.default", 1.0), + ([-0.0, -0.0], "aten.max.default", -1.0), +] + + +def _sign(x): + return math.copysign(1.0, x) + + +def _shim(values, dtype): + return _C._tensor_from_flat(values, [len(values)], _DTYPES[dtype]) + + +def test_min_max_default_order_negative_zero_first(): + problems = [] + for dtype in _DTYPES: + for values, op, want in _CASES: + got = _C._aten_dispatch(op, _shim(values, dtype)).tolist() + if got != 0.0 or _sign(got) != want: + problems.append("%s(%s) %s = %r, upstream %r" + % (op, values, dtype, got, math.copysign(0.0, want))) + assert not problems, "\n".join(problems) + + +def test_min_max_dim_keep_the_first_equal_element(): + # `max.dim`/`min.dim` answer the element at the first equal index: + # `min([0., -0.], dim=0)` is `0.0` and `max([-0., 0.], dim=0)` is `-0.0` + # upstream (measured). The whole-tensor rule must not leak into them. + problems = [] + for dtype in _DTYPES: + for values, op in (([0.0, -0.0], "aten.min.dim"), ([-0.0, 0.0], "aten.max.dim")): + got = _C._aten_dispatch(op, _shim(values, dtype), 0) + value, index = got[0].tolist(), got[1].tolist() + if index != 0 or _sign(value) != _sign(values[0]): + problems.append("%s(%s) %s = (%r, %r), upstream (%r, 0)" + % (op, values, dtype, value, index, values[0])) + assert not problems, "\n".join(problems) + + +@_skip.known_x86_64_linux_divergence( + "#40", "the whole-tensor min/max zero sign: the table and the shim follow arm64 " + "upstream; x86_64 upstream answered min([-0., 0., -0., 0.]) = +0.0 on the hosted " + "runner (PR #83)") +def test_the_pinned_answers_are_upstreams(): + """Re-asks upstream in this process, so the table above is not a value + written down once. Skipped by name when upstream is not importable (a + shim-only interpreter), or when `torch` here is the shim.""" + if _upstream_torch is None or hasattr(_upstream_torch._C, "_aten_implemented"): + _skip.skip(" (skipped test_the_pinned_answers_are_upstreams: no upstream torch " + "in this interpreter)") + return + problems = [] + for dtype in _DTYPES: + for values, op, want in _CASES: + t = _upstream_torch.tensor(values, dtype=getattr(_upstream_torch, dtype)) + got = (t.min() if op == "aten.min.default" else t.max()).item() + if _sign(got) != want: + problems.append("upstream %s(%s) %s = %r, table says sign %r" + % (op, values, dtype, got, want)) + assert not problems, "\n".join(problems) + + +def _main(): + items = [(name, fn) for name, fn in sorted(globals().items()) + if name.startswith("test_")] + failures = _skip.run_tests(items, suite="test_signedzero") + return 1 if failures else 0 + + +if __name__ == "__main__": + raise SystemExit(_main()) diff --git a/torchnative/rust/torch_c/src/aten.rs b/torchnative/rust/torch_c/src/aten.rs index 8c6664f8..ce33f84c 100644 --- a/torchnative/rust/torch_c/src/aten.rs +++ b/torchnative/rust/torch_c/src/aten.rs @@ -10997,6 +10997,18 @@ fn promote_operands(op: &str, lhs: &PyTensorBase, rhs: &PyTensorBase) -> PyResul if let Some(err) = float8_promotion_refusal(lhs.tag(), rhs.tag()) { return Err(err); } + // c10's `ResultTypeState` (issue #93): a tensor with dimensions outranks a + // 0-dim tensor within the same category, so `float32[2] + tensor(0.1, + // float64)` is float32 upstream, not float64, while an integral tensor + // against a 0-dim float64 still promotes to float64. Only when exactly + // one side is 0-dim does the rule differ from `promote_types`. + let (l0, r0) = (lhs.dims().is_empty(), rhs.dims().is_empty()); + if l0 != r0 { + let (dim, zero) = if l0 { (rhs.tag(), lhs.tag()) } else { (lhs.tag(), rhs.tag()) }; + if let Some(tag) = combine_categories(dim, zero) { + return Ok(tag); + } + } promote_types(lhs.tag(), rhs.tag()).ok_or_else(|| { not_implemented(format!( "{op}: dtype promotion not implemented in torch._C shim: {} vs {}", @@ -11006,6 +11018,32 @@ fn promote_operands(op: &str, lhs: &PyTensorBase, rhs: &PyTensorBase) -> PyResul }) } +/// c10's `combine_categories(higher, lower)` (ScalarType.h / TypeProperties.cpp), +/// for a dimensioned operand `higher` and a 0-dim operand `lower`. `None` +/// falls back to `promote_types` (the cases where c10 promotes the two). +fn combine_categories(higher: TorchDType, lower: TorchDType) -> Option { + use TorchDType::*; + if higher.is_complex() { + return Some(higher); + } + if lower.is_complex() { + // A real floating `higher` keeps its precision in the complex result. + return match higher { + Float16 => Some(Complex32), + Float32 => Some(Complex64), + Float64 => Some(Complex128), + _ => None, + }; + } + if higher.is_floating_point() { + return Some(higher); + } + if higher == Bool || lower.is_floating_point() { + return None; + } + Some(higher) +} + /// An operand cast **to the common dtype**, which is the step that has to /// happen before any widening to an accumulator. /// @@ -11154,11 +11192,47 @@ fn div_scalar_reduced_float( /// measured 57x slower than upstream's `amax` at this shape -- 24.3% of a /// `float32` prefill (docs/numerics/SEQLEN.md §7). That is a *speed* argument, and the /// two are documented to agree **bit for bit on every input this line can -/// produce** (§7.2, an argument rather than a tolerance), so falling back to -/// the reduction candle does have on Metal changes nothing but the time. +/// produce** (§7.2, an argument rather than a tolerance). That argument was +/// made for candle's *CPU* reduction; its Metal one is not that function, +/// which is what the correction below is for. +/// +/// **On Metal a float input is not handed to `max_keepdim` alone** (issue +/// #49). candle's Metal reduce is seeded with `numeric_limits::lowest()`, +/// the finite `-3.4e38` (f16: `-65504`), and skips NaN, so `amax` of an +/// all-`-inf` row answered `-FLT_MAX` and `amax([nan, 1])` answered `1.0`. The +/// value is instead *gathered* at the index `first_extremum_index` would +/// report, built the same way in candle ops on the device: +/// +/// 1. `extreme = max_keepdim(x)` (possibly the seed); +/// 2. `first_hit = argmax_keepdim(x == extreme)`: index 0 when `extreme` was +/// the seed because the row is all `-inf`, and element 0 *is* `-inf`; +/// 3. the first NaN's index wherever the row holds a NaN (`x != x`); +/// 4. `x.gather(index)`: `-inf`, `nan` and ties come out as the elements they +/// are. An integer input has no infinity or NaN and its seed is its true +/// minimum, so it takes `max_keepdim` directly. +/// +/// Softmax's row max (`_softmax`, `logsumexp`) goes through here too. Its +/// *answers* did not change: a row of all `-inf` is NaN either way and a NaN +/// row is NaN either way. What the extra passes buy is that the row max is the +/// true one. fn amax_keepdim_anywhere(t: &Tensor, dim: usize) -> candle_core::Result { if crate::device::is_metal(t.device()) { - return t.max_keepdim(dim); + if !t.dtype().is_float() { + return t.max_keepdim(dim); + } + let t = t.contiguous()?; + let extreme = t.max_keepdim(dim)?; + let first_hit = t + .broadcast_eq(&extreme)? + .argmax_keepdim(dim)? + .to_dtype(candle_core::DType::I64)?; + let nan_flags = t.ne(&t)?; + let any_nan = nan_flags.max_keepdim(dim)?; + let first_nan = nan_flags + .argmax_keepdim(dim)? + .to_dtype(candle_core::DType::I64)?; + let index = any_nan.where_cond(&first_nan, &first_hit)?; + return t.gather(&index, dim); } crate::tensor::amax_keepdim(t, dim) } @@ -11577,6 +11651,26 @@ fn compare_common(op: &str, tensor: &Tensor, floating: bool, tag: TorchDType) -> // kernel name (`lt_cpu` and friends) and the table at the door raises that // before any operand reaches here. if floating { + // **Off the CPU the comparison runs in the common dtype itself** + // (issue #49). Metal has no `f64`, so the widening below died with + // `Metal contiguous to_dtype F32 F64 not implemented` for every float + // `eq`/`ne`/`lt`/... on `mps`, an empty tensor included. The widening + // was a convenience for the CPU's one-kernel path and never part of + // the answer: both operands already hold the common dtype + // (`compare_tensor`'s `operand_in`), and comparing two values of one + // dtype in that dtype or in a wider one gives the same booleans. + if !tensor.device().is_cpu() { + return device_compare_operand(op, tensor); + } + // An integral or `bool` operand meeting a float scalar compares in the + // default float dtype upstream (issue #93): `int64([16777217]) == + // 16777216.0` is True because both sides become `float32` first. + // Widening it straight to `f64` would keep the two apart. + if !tensor.dtype().is_float() { + let storage = PyDtype::new(default_float()).storage(op)?; + let cast = tensor.to_dtype(storage).map_err(|e| candle_err(op, e))?; + return widen_f64(&cast).map_err(|e| candle_err(op, e)); + } return widen_f64(tensor).map_err(|e| candle_err(op, e)); } tensor @@ -11584,6 +11678,22 @@ fn compare_common(op: &str, tensor: &Tensor, floating: bool, tag: TorchDType) -> .map_err(|e| candle_err(op, e)) } +/// A float comparison operand that is not on the CPU, kept on its device. +/// +/// A float operand stays in its own dtype. An integral or `bool` operand +/// meeting a float scalar is upstream's integer-with-float-scalar case, whose +/// comparison dtype is the default float, so it is cast to that on the device +/// (`metal_dtype_gate` refuses a default dtype the device does not have, by +/// name, rather than letting candle die on a missing cast symbol). +fn device_compare_operand(op: &str, tensor: &Tensor) -> PyResult { + if tensor.dtype().is_float() { + return Ok(tensor.clone()); + } + let storage = PyDtype::new(default_float()).storage(op)?; + crate::device::metal_dtype_gate(tensor.device(), storage)?; + tensor.to_dtype(storage).map_err(|e| candle_err(op, e)) +} + fn apply_cmp(op: &str, kind: Cmp, lhs: &Tensor, rhs: &Tensor) -> PyResult { match kind { Cmp::Eq => lhs.broadcast_eq(rhs), @@ -11645,8 +11755,25 @@ fn compare_scalar( scalar_arg(op, args, kwargs, 1, "other")?.ok_or_else(|| missing(op, "other"))?; let floating = lhs.tag().is_floating_point() || !other.is_int(); let left = compare_common(op, lhs.tensor()?, floating, lhs.tag())?; - let right = if floating { - host_const(other.as_f64(), &[], left.device()) + let right = if floating && !left.device().is_cpu() { + // Narrowed to the comparison dtype on the host, which is upstream's + // rule for a Python scalar meeting a float tensor (`float16(0.1) == + // 0.1` is `True` upstream: the scalar is cast to `float16`). It is also + // the only constant Metal can hold: `host_const` refuses an `f64` one. + host_const(other.as_f64(), &[left.dtype()], left.device()) + } else if floating { + // The CPU compares in `f64`, but the scalar is first narrowed to the + // comparison dtype, upstream's rule (issue #93): `float32([0.1]) == + // 0.1` is True upstream because 0.1 is cast to `float32`, and + // `float16([inf]) == 65520.0` is True because 65520 rounds to `inf` in + // `float16`. The comparison dtype is the tensor's own float dtype, or + // the default float for an integral/`bool` tensor (`compare_common` + // casts that operand the same way). `float_narrower` is c10's + // rounding, so f16/bf16 agree bit for bit; narrowing is exact to a + // representable value, so comparing the two in `f64` afterwards gives + // the comparison dtype's booleans. + let ctag = if lhs.tag().is_floating_point() { lhs.tag() } else { default_float() }; + host_const(float_narrower(ctag)(other.as_f64()), &[], left.device()) } else { host_const(wrap_unsigned_scalar(other.as_i64(), lhs.tag()), &[], left.device()) } @@ -13990,11 +14117,60 @@ fn extremum_default( .and_then(|v| v.to_dtype(storage)), _ => flat.index_select(&index, 0), } - .and_then(|v| v.reshape(())) .map_err(|e| candle_err(op, e))?; + let out = if tag.is_floating_point() && flat.device().is_cpu() { + signed_zero_extremum(op, &flat, &out, which)? + } else { + out + }; + let out = out.reshape(()).map_err(|e| candle_err(op, e))?; finish(py, out, tag) } +/// `picked` (shape `[1]`, the element `first_extremum_index` chose) with the +/// sign of a zero answer corrected to upstream's rule (issue #49). +/// +/// The whole-tensor `min()`/`max()` order `-0.0 < +0.0`: `min` answers `-0.0` +/// whenever a `-0.0` is present and `max` answers `+0.0` whenever a `+0.0` is, +/// wherever it sits (measured on torch 2.13.0 for every position of lengths +/// 1..69 in float32, float16 and bfloat16). The `dim=` overloads do not -- +/// they answer the first equal element, which is what `picked` already is -- +/// so this is called from `extremum_default` only. `min([0., -0., 0., -0.])` +/// answered `0.0` here before it. +/// +/// The sign of a zero is read as the sign of its reciprocal (`1/-0 = -inf`), +/// the one arithmetic test that tells the two apart; `x < 0` cannot. Every +/// step is a candle op on the tensor's device, but the caller runs it on the +/// CPU only: candle compiles its Metal kernels with fast math on, under which +/// `1/0` is not specified, and a wrong sign test would turn a correct `+0.0` +/// into `-0.0`. On `mps` the sign of a zero answer stays the first equal +/// element's (not measured against upstream mps). +fn signed_zero_extremum( + op: &str, + flat: &Tensor, + picked: &Tensor, + which: Extremum, +) -> PyResult { + let fixed = (|| -> candle_core::Result { + let is_zero = flat.eq(0f64)?; + let reciprocal = flat.recip()?; + let preferred_sign = match which { + Extremum::Min => reciprocal.lt(0f64)?, + Extremum::Max => reciprocal.gt(0f64)?, + }; + // u8 0/1 flags, `*` is "and"; the `max` over them is "any". + let present = is_zero.mul(&preferred_sign)?.max_keepdim(0)?; + let answer_is_zero = picked.eq(0f64)?; + let apply = present.mul(&answer_is_zero)?; + let signed = match which { + Extremum::Min => picked.abs()?.neg()?, + Extremum::Max => picked.abs()?, + }; + apply.where_cond(&signed, picked) + })(); + fixed.map_err(|e| candle_err(op, e)) +} + /// `aten::amax(Tensor self, int[1] dim=[], bool keepdim=False) -> Tensor` /// /// The maximum *value* over some dimensions, with no indices -- which is what @@ -17534,7 +17710,11 @@ fn to_copy_default( .tensor()? .to_device(&device) .map_err(|e| candle_err(OP, e))?; - let out = if moved.dtype().is_int() { + // A float tensor off the CPU is compared in its own dtype too (issue + // #49): `widen_f64` there is `Metal contiguous to_dtype F32 F64 not + // implemented`, an empty tensor included, and `x != 0` does not depend + // on the width (NaN is non-zero at every width). + let out = if moved.dtype().is_int() || !moved.device().is_cpu() { moved.ne(0f64).map_err(|e| candle_err(OP, e))? } else { widen_f64(&moved)