test_metal_mpp_equivalence can pass without varying the Metal4 route (non-discriminating on CUDA)

[marcodelpin]

Summary

test_metal_mpp_equivalence is meant to compare a Metal4/TensorOps candidate against a legacy
reference. It never checks that the candidate arm actually took a different route, and there are
ordinary configurations, including on macOS, where it does not. When that happens the case can still
pass, and its log line still reads like an equivalence result.

The clearest instance is a CUDA build, where no compiled backend code reads the variable that is
supposed to separate the arms.

How the arms are separated

    char *saved_disable_metal4 = test_save_env("DS4_METAL_DISABLE_METAL4");
    setenv("DS4_METAL_DISABLE_METAL4", "1", 1);
    ds4_engine *ref_engine = test_open_engine(false);
    ...
    ds4_engine_close(ref_engine);
    test_restore_env("DS4_METAL_DISABLE_METAL4", saved_disable_metal4);

    test_run_mpp_candidate("auto", cases, ncase);

(tests/ds4_test.c:5732-5755)

The reference is forced to 1. The candidate is not forced to anything: the caller's previous value
is restored, and the candidate is whatever that yields.

There is a route= field in the output that looks like it reports this, but it does not:

static void test_run_mpp_candidate(const char *label,
                                   test_mpp_eq_case *cases,
                                   int ncase) {
    fprintf(stderr, "ds4-test: Tensor equivalence candidate route=%s\n", label);

(tests/ds4_test.c:5683-5686)

label is caller-supplied, and the single call site passes the literal "auto"
(tests/ds4_test.c:5755), so the line prints route=auto whatever actually ran.

The two arms therefore differ only if the restored environment and the machine and the specific
workload all end up selecting the alternate path. They do not differ when:

• the caller already had DS4_METAL_DISABLE_METAL4 set to a disabling value (both arms disabled,
  on macOS too. Note 0/false/no/off parse as not disabling, so this is narrower than "set");
• the Metal4/TensorOps path is unavailable at runtime: unsupported hardware, older SDK/runtime, or a
  failed capability probe;
• the workload's tensor shapes/types do not select an alternate dispatch, or the specific alternate
  pipeline falls back to the legacy kernel anyway. This one can happen even when Metal4 is globally
  available, which is why availability is not the right thing to test;
• the test is run with DS4_TEST_BACKEND=cpu, which bypasses Metal even on a build that has it;
• the Metal backend is not in the build at all, which is the CUDA case below.

The CUDA case

• the only non-test reader of the variable is ds4_metal.m:2470:

  const int metal4_disabled = ds4_gpu_env_bool("DS4_METAL_DISABLE_METAL4") > 0;

• Makefile:27 puts ds4_metal.o in CORE_OBJS only inside ifeq ($(UNAME_S),Darwin); the else
  branch (Makefile:53) uses ds4_cuda.o instead. Ordinary native CUDA builds therefore do not
  compile that Objective-C source and do not provide the Metal framework it imports at
  ds4_metal.m:2.
• the strix-halo ROCm invocation (Makefile:180-181) likewise omits ds4_metal.o from its
  CORE_OBJS override, though that target builds only the production binaries, not ds4_test, so
  this is a source-level observation about the object list rather than a ROCm test run I have
  exercised.

On such a build both engines are configured identically, so the case becomes a same-route
repeatability comparison rather than a Metal4-versus-legacy one, and the line at
tests/ds4_test.c:5528

    fprintf(stderr,
            "ds4-test: Tensor equivalence %s top1 ref=%d cand=%d top5_overlap=%d/%d overlap=%d/%d max_rank_delta=%d rms=%g max_abs=%g top20_max_abs=%g\n",

can render with zero deltas throughout when the repeated execution agrees.

Worth stating carefully, because it cuts both ways. The arms are separately initialised engines
running separate inference, so zeros there are an observed result, not a structural identity.
CUDA has floating-point accumulation paths where execution order can matter. And the logit-comparison
pass flag is nonfinite == 0 && same_top1 (tests/ds4_test.c:5524), so nonzero rms, max-abs and rank
deltas do not by themselves fail anything; the separate greedy-output assertions described below
still have to hold. Either way nothing here establishes cross-variant equivalence, because no variant
was varied.

What a pass still means off the Metal4 path

Not nothing. The normal execution assertions still run (engine and session creation, synchronisation,
logit copying, evaluation, allocation, case loading), so it remains a real inference smoke test. The
comparison-specific assertions are:

    if (assert_thresholds) {
        TEST_ASSERT(nonfinite == 0);
        TEST_ASSERT(same_top1);
    }

plus cand_gen_len == tc->ref_gen_len and the per-step cand_gen[j] == tc->ref_gen[j] in
test_run_mpp_candidate. nonfinite == 0 is a genuine independent check; the rest now compare two
runs of the same configuration. So off the Metal4 path the comparison degrades to greedy-output
repeatability plus logit finiteness. Note that full-logit equality is logged but not asserted.

That is worth testing. The problem is only that it is filed under a name, and a log line, that claim
something stronger, so a passing Tensor equivalence ... rms=0 max_abs=0 reads to a CUDA
contributor as "my change preserved numerics", which it cannot show.

Suggestion

Key the case on whether the candidate arm actually dispatched the alternate route, recorded per
case rather than as a global availability flag, and report N/A (or skip) when it did not. That
covers the caller-disabled, unsupported-hardware, shape-fallback and not-compiled cases uniformly.

Conveniently the reporting slot already exists: populating route= with the observed route instead
of the literal "auto" would make the degenerate runs self-evident in the log even before any
skip logic is added.

Explicitly enabling Metal4 for the candidate is worth doing as well, but it is not sufficient on its
own, because clearing the disable switch cannot activate the path on unsupported hardware, an older SDK, or
after a failed probe, and those runs would go on passing without varying anything.

Happy to send a patch if you say which shape you prefer.

Environment / how to reach it

sm_110 CUDA build (Jetson AGX Thor T5000, CUDA 13.0.48, L4T R38.4, aarch64). Note make cuda does
not build the test binary. It builds ds4 ds4-server ds4-bench ds4-eval ds4-agent (Makefile:177);
ds4_test is a separate target, and the case is selected with --metal-tensor-equivalence
(tests/ds4_test.c:6432).

Everything above is established by reading the source and the Makefile, which is why I have cited
line numbers rather than pasted a run: the conclusion does not depend on any particular execution.