antirez / antirez/ds4

ds4-agent advertises ROCm backend options but rejects them on a Strix Halo build

Ouverte
#829 3 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
C
Étoiles
22.3k
Forks
2.1k
Merge moyen
1 j 3 h
PR mergées (30 j)
4

Description

## Summary

On a `make strix-halo` ROCm build, `ds4-agent --help` correctly advertises
`--rocm` and `--backend rocm`, but `ds4-agent` rejects both options.

The agent itself is not blocked from running on ROCm. The undocumented `--cuda`
spelling currently works, as does omitting the backend option entirely. The
issue is that the public CLI advertised by the shared help does not match the
agent's argument parser.

## Environment

- Framework Desktop
- AMD Ryzen AI Max+ 395
- AMD Radeon 8060S (`gfx1151`)
- 128 GB RAM
- Fedora 44
- Kernel 7.1.8-200.fc44.x86_64
- ROCm 7.1.x
- Build target: `make strix-halo`
- Model: DeepSeek V4 Flash 0731 `ds4f-q2`
- Model size: 80.76 GiB

Build note: on Fedora this build needs the previously reported linker workaround
to link against the Fedora ROCm packages. Without it `make strix-halo` does not
produce binaries on this system, so it is required to reach the behaviour below:

```sh
make strix-halo -j"$(nproc)" \
ROCM_CFLAGS="-O3 -ffast-math -g -fno-finite-math-only -pthread -D__HIP_PLATFORM_AMD__ -Wno-unused-command-line-argument --offload-arch=gfx1151 -no-pie" \
ROCM_LDLIBS="-lm -pthread -lhipblas -lhipblaslt -lamdhip64"
```

The normal `ds4` CLI is working correctly with ROCm on this machine:

```sh
./ds4 --rocm -p "Reply with exactly: DS4 is working."
```

produces, among other output:

```text
ds4: ROCm backend initialized on AMD Radeon 8060S Graphics (sm_115)
ds4: ROCm preparing model tensor mappings: 80.24 GiB
ds4: ROCm startup model preparation covered 80.76 GiB of tensor spans
DS4 is working.
ds4: prefill: 34.32 t/s, generation: 16.34 t/s
```

## Reproduction

`ds4-agent --help` advertises:

```text
--metal | --rocm | --cpu
--backend NAME Backend name: metal, rocm, or cpu.
```

Both advertised spellings are rejected, each with exit status 2, but by two
different code paths — worth noting because they need two separate fixes:

```sh
$ ./ds4-agent --rocm --help
ds4-agent: unknown option: --rocm
ds4-agent
Run the native terminal coding agent with live tools, session save/restore, ...
[... full help follows, 57 lines total ...]
$ echo $?
2
```

`--rocm` falls through the argument loop, so the unknown-option message is
followed by the normal help output.

```sh
$ ./ds4-agent --backend rocm --help
ds4-agent: invalid backend: rocm
$ echo $?
2
```

`--backend rocm` exits inside `parse_backend()` before help renders — one line
of output total. Note it also prints no list of valid backend names, unlike
`ds4_cli.c:225-230` and `ds4_server.c:13058-13063`, which both print a
build-appropriate `valid backends are: ...` line.

## Current workarounds

The agent is capable of using the ROCm build today.

This works:

```sh
./ds4-agent --cuda --help
```

because `ds4_agent.c:668` accepts `--cuda` unconditionally and maps it to
`DS4_BACKEND_CUDA`, which is also the internal backend enum used by the ROCm
build.

Likewise, omitting the backend option works because `default_backend()`
(`ds4_agent.c:535-543`) returns `DS4_BACKEND_CUDA` on non-Apple GPU builds.

So this is not "ds4-agent cannot run on ROCm"; it is specifically an
inconsistency between the advertised ROCm CLI and the agent's parser.

## Cause

The shared help and the per-frontend backend parsers have diverged.

The backend help is generated centrally by `ds4_help.c`
(`print_model_runtime()`, lines 150-161) and correctly uses `DS4_ROCM_BUILD` to
advertise the appropriate backend spelling for the build.

Backend parsing, however, is duplicated across the individual frontends. Four of
the five copies have the ROCm conditional; the agent's does not:

```sh
$ grep -c DS4_ROCM_BUILD ds4_cli.c ds4_server.c ds4_bench.c ds4_eval.c ds4_help.c ds4_agent.c
ds4_cli.c:6
ds4_server.c:3
ds4_bench.c:3
ds4_eval.c:3
ds4_help.c:1
ds4_agent.c:0
```

`ds4-agent` is the only frontend with no ROCm conditional anywhere.

Its `parse_backend()` (`ds4_agent.c:527-533`) accepts:

```text
metal
cuda
cpu
```

and its option parser (`ds4_agent.c:666-669`) handles:

```text
--metal
--cuda
--cpu
```

but neither handles `rocm` / `--rocm`.

For comparison, `ds4_cli.c:217-232` and `ds4_cli.c:1940-1948` map `rocm` and
`--rocm` to the existing `DS4_BACKEND_CUDA` enum under `DS4_ROCM_BUILD`.

This looks like the agent parser did not receive the ROCm CLI changes the other
frontends received, while the shared help automatically started advertising the
ROCm interface.

## Related test issue

While investigating, I found that `tests/test_gpu_args_cli.sh` — which runs as
part of `make test` — is already failing on a ROCm/Strix Halo build, independent
of the agent bug:

```text
test_gpu_args_cli: PASS=32 FAIL=12
```

The 12 failures are these three, repeated for `ds4`, `ds4-server`, `ds4-bench`,
and `ds4-agent`:

```text
FAIL ds4 --help mentions --gpu-vram
FAIL ds4 --help mentions --gpu-devices
FAIL ds4 --help mentions --cuda-tensor-parallel
```

Test 1 (`tests/test_gpu_args_cli.sh:38-54`) expects these CUDA-specific options
in every binary's help, while `ds4_help.c:153-161` deliberately omits all three
under `DS4_ROCM_BUILD`. So `make test` cannot currently be green on Strix Halo.

There is a second, subtler problem with test 4, the `--cuda --help` check
(`tests/test_gpu_args_cli.sh:107-117`). It asserts only that the output contains
`Usage:`, `usage:` or `--help`. On a ROCm build it passes for all four binaries,
for two different reasons:

| Binary | `--cuda` on a ROCm build | Why test 4 passes |
| --- | --- | --- |
| `ds4`, `ds4-server`, `ds4-bench` | rejected (`unknown option: --cuda`) | error, then help still prints — vacuous pass |
| `ds4-agent` | accepted | genuine pass |

The assertion therefore reports green while the four binaries actively disagree
about whether `--cuda` exists. That is the exact drift this issue is about, and
the existing coverage cannot detect it.

This seems relevant to the QA requirement in `QA_BEFORE_RELEASES.md` §2, that an
`unknown option` response from a binary that advertises the flag is a release
blocker. The policy exists; the automated check just cannot enforce it on ROCm.

It probably also explains how the `ds4-agent` discrepancy escaped the existing
CLI regression coverage: that suite does not produce a clean or meaningful
result on a ROCm build, so its output is not read on this platform.

Separately, `ds4-eval` is not covered by this test at all
(`tests/test_gpu_args_cli.sh:34` lists only `ds4`, `ds4-server`, `ds4-bench`,
`ds4-agent`), despite having its own duplicated `parse_backend()` at
`ds4_eval.c:1483`. Worth closing in the same pass, or the next instance of this
drift lands in `ds4-eval` and goes unnoticed the same way.

## Proposed fix

There appear to be two reasonable scopes.

### Option A: minimal fix

Mirror the existing ROCm handling from the other frontends in `ds4_agent.c`.

Under `DS4_ROCM_BUILD`:

- accept `--rocm`;
- accept `--backend rocm`;
- map both to the existing `DS4_BACKEND_CUDA` internal backend;
- expose the `cuda` spelling only on non-ROCm builds;
- add the build-appropriate valid-backend diagnostic already used by the other
frontend parsers.

No new backend enum is required.

**Deliberate break to confirm:** the fourth bullet removes the `--cuda`
workaround documented above. That is intentional, for parity with `ds4_cli.c`,
which already rejects `--cuda` on a ROCm build — but it does change behaviour
for anyone currently scripting `--cuda` against `ds4-agent` on ROCm. If that is
not wanted, the alternative is to keep `--cuda` accepted as an undocumented
alias on ROCm builds and only add the `rocm` spellings. Happy to go either way;
flagging it so the choice is explicit rather than incidental.

I would also make `tests/test_gpu_args_cli.sh` build-aware — assert the
ROCm-appropriate help under `DS4_ROCM_BUILD` instead of the CUDA options — and
add a regression check that an advertised backend option is actually *accepted*,
rather than merely checking that help text appears somewhere in the output.
Adding a ROCm assertion to the script in its current state would land in a suite
that is already red on this platform, so the build-awareness fix should come
first.

### Option B: centralise backend-name parsing

The underlying cause is that backend-name parsing is duplicated across the five
frontends, and this is the fifth copy having drifted.

Since `ds4_gpu_args.c` already provides shared GPU argument handling — and
`QA_BEFORE_RELEASES.md` §2 already treats it as the parser all binaries must
route through — an alternative is to introduce a shared backend-name parse and
valid-names helper there, and have the frontends delegate to it.

That is a larger change touching five files, but it removes this source of
frontend drift rather than fixing the fifth copy independently.

I am happy to submit and test a patch for either scope on the Framework Desktop
/ Strix Halo system.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.