antirez / antirez/ds4

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

Đang mở
#829 3 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C
Star
22.3k
Fork
2.1k
Merge trung bình
1 ngày 3 giờ
Pull request đã merge (30 ngày)
4

Mô tả

## 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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.