Skip to content

Commit 305ce3c

Browse files
committed
feat: add setFlashAttn(FlashAttn), deprecate the valueless enableFlashAttn
llama.cpp turned --flash-attn from a bare flag into a value-taking option in b10273: on|off|auto is mandatory. enableFlashAttn() emits the key with no value, so the parser consumes whatever argv token follows and the load dies naming a flag the caller never set -- error: unknown value for --flash-attn: '--reasoning-format' is a real observed message. Until now the option was simply not expressible through this binding. putScalar is protected, so a downstream caller could not work around it either; srcmorph ships a plan-time refusal for exactly this reason. args.FlashAttn follows the CacheType / TensorReadLazyMode pattern, and setFlashAttn routes through putEnum, so the value lands as its own argv token after the key. enableFlashAttn() is deprecated rather than fixed in place. Rewriting it to emit "on" would turn Flash Attention on for every caller whose argv happened to survive -- a behaviour change wearing a bugfix's clothes. Error Prone's @InlineMe suggestion is declined for the same reason and the suppression says so: inlining would copy the broken emission into callers, which is the opposite of the migration, and the replacement is not equivalent because it takes a value the old method cannot supply. One existing test had encoded the defect as the contract: testToArrayComplexCombination asserted a 9-token argv built with enableFlashAttn(). It now uses the new setter and asserts 10, and the deprecated method's valueless emission gets its own test that says in its name and javadoc that it is pinning a defect. Two further tests cover every enum value and assert the value is the token immediately after the key -- the property whose absence caused all of this. llama module: 1478 tests, 0 failures.
1 parent 6e1c66f commit 305ce3c

4 files changed

Lines changed: 137 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,30 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by
99

1010
## [Unreleased]
1111

12+
### Added
13+
- **`ModelParameters.setFlashAttn(FlashAttn)` — the only way to express `--flash-attn` correctly.**
14+
llama.cpp turned that option from a bare flag into a value-taking one in **b10273**: the
15+
`on|off|auto` value is mandatory, so emitting the key alone makes the parser consume whatever argv
16+
token happens to follow it. The failure is as misleading as it sounds — the load dies naming a flag
17+
the caller never set, e.g. `error: unknown value for --flash-attn: '--reasoning-format'`.
18+
19+
The new `args.FlashAttn` enum follows the existing `CacheType` / `TensorReadLazyMode` pattern, so
20+
the option is now expressible: `AUTO` is upstream's own default, `ON` forces it and fails the load
21+
where the backend cannot provide it, `OFF` disables it.
22+
23+
### Deprecated
24+
- **`ModelParameters.enableFlashAttn()`** — it emits the valueless `--flash-attn` described above.
25+
Its behaviour is deliberately left unchanged rather than quietly rewritten to emit `on`: that would
26+
turn Flash Attention *on* for callers whose argv happened to survive, which is a behaviour change
27+
disguised as a fix. Migrate to `setFlashAttn(FlashAttn)`.
28+
29+
### Fixed
30+
- **A test pinned the broken argv shape as correct.** `ModelParametersExtendedTest`'s
31+
complex-combination case asserted a 9-token argv built with `enableFlashAttn()` — i.e. it encoded
32+
the valueless emission as the expected contract, which is why no gate ever flagged it. It now uses
33+
`setFlashAttn(FlashAttn.ON)` and asserts 10 tokens, and a separate test pins the deprecated
34+
method's emission explicitly as the defect it is, so the two cannot be confused again.
35+
1236
## [5.1.0] - 2026-08-29
1337

1438
> The entries below also cover the **b9917 → b10456** window (PRs #341–#394), which went unrecorded
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// SPDX-FileCopyrightText: 2026 Bernard Ladenthin <bernard.ladenthin@gmail.com>
2+
// SPDX-FileCopyrightText: 2023-2025 Konstantin Herud
3+
//
4+
// SPDX-License-Identifier: MIT
5+
6+
package net.ladenthin.llama.args;
7+
8+
/**
9+
* Flash Attention mode for {@code --flash-attn}.
10+
*
11+
* <p>llama.cpp turned {@code --flash-attn} from a bare flag into a value-taking option in b10273:
12+
* the value is mandatory, and emitting the key alone makes the parser consume whatever argv token
13+
* follows it. That is why this is an enum rather than a boolean — see
14+
* {@link net.ladenthin.llama.parameters.ModelParameters#setFlashAttn(FlashAttn)}.</p>
15+
*/
16+
public enum FlashAttn implements CliArg {
17+
18+
/** Force Flash Attention on; the model load fails if the backend cannot provide it. */
19+
ON("on"),
20+
/** Force Flash Attention off. */
21+
OFF("off"),
22+
/** Let llama.cpp decide per backend and model — upstream's own default. */
23+
AUTO("auto");
24+
25+
private final String argValue;
26+
27+
FlashAttn(String argValue) {
28+
this.argValue = argValue;
29+
}
30+
31+
@Override
32+
public String getArgValue() {
33+
return argValue;
34+
}
35+
}

‎llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,14 +250,46 @@ public ModelParameters disableContextShift() {
250250
}
251251

252252
/**
253-
* Enable Flash Attention (default: disabled).
253+
* Enable Flash Attention by emitting {@code --flash-attn} with no value.
254+
*
255+
* <p><b>Deprecated because it produces an argv llama.cpp misreads.</b> Since b10273
256+
* {@code --flash-attn} takes a mandatory {@code on|off|auto} value, so a valueless emission makes
257+
* the parser consume the <em>next</em> argv token as the mode. The model load then fails naming a
258+
* flag the caller never set &mdash; for example {@code error: unknown value for --flash-attn:
259+
* '--reasoning-format'} &mdash; which is as misleading a diagnostic as it looks.</p>
260+
*
261+
* <p>Use {@link #setFlashAttn(FlashAttn)} instead. This method is kept so existing callers still
262+
* compile, and its behaviour is deliberately unchanged: silently rewriting it to emit
263+
* {@code on} would flip Flash Attention on for callers whose argv happened to survive, which is a
264+
* behaviour change disguised as a fix.</p>
254265
*
255266
* @return this builder
267+
* @deprecated emits a valueless {@code --flash-attn}; use {@link #setFlashAttn(FlashAttn)}
256268
*/
269+
@Deprecated
270+
// Error Prone offers @InlineMe for a deprecated one-liner. Declined on purpose: inlining would
271+
// copy setFlag(ModelFlag.FLASH_ATTN) -- the broken emission itself -- into every caller, which is
272+
// the opposite of the migration. The replacement is setFlashAttn(FlashAttn), and it is not
273+
// equivalent: it takes a value this method cannot supply.
274+
@SuppressWarnings("InlineMeSuggester")
257275
public ModelParameters enableFlashAttn() {
258276
return setFlag(ModelFlag.FLASH_ATTN);
259277
}
260278

279+
/**
280+
* Set the Flash Attention mode ({@code --flash-attn}).
281+
*
282+
* <p>The value is mandatory upstream, so this is the only way to express the option correctly.
283+
* {@link FlashAttn#AUTO} is llama.cpp's own default and lets it decide per backend and model;
284+
* {@link FlashAttn#ON} forces it and fails the load where the backend cannot provide it.</p>
285+
*
286+
* @param mode the Flash Attention mode
287+
* @return this builder
288+
*/
289+
public ModelParameters setFlashAttn(FlashAttn mode) {
290+
return putEnum("--flash-attn", mode);
291+
}
292+
261293
/**
262294
* Use the full-size SWA KV cache so the sliding-window layers' KV is reusable across requests
263295
* (restores prompt-prefix cache reuse with {@link #setCacheReuse(int)}); costs ~2x SWA-layer

‎llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import net.ladenthin.llama.ClaudeGenerated;
2323
import net.ladenthin.llama.args.*;
2424
import net.ladenthin.llama.args.CacheType;
25+
import net.ladenthin.llama.args.FlashAttn;
2526
import net.ladenthin.llama.args.GpuSplitMode;
2627
import net.ladenthin.llama.args.MiroStat;
2728
import net.ladenthin.llama.args.NumaStrategy;
@@ -634,13 +635,50 @@ public void testDisableContextShift() {
634635
assertThat(p.parameters.get("--no-context-shift"), is(nullValue()));
635636
}
636637

638+
/**
639+
* Pins the DEPRECATED method's emission -- a key with a null value, i.e. a valueless
640+
* {@code --flash-attn} -- as the defect it is, not as the contract.
641+
*
642+
* <p>llama.cpp has required a mandatory {@code on|off|auto} since b10273, so this argv makes the
643+
* parser eat the next token. The behaviour is kept deliberately (changing it would flip Flash
644+
* Attention on for callers whose argv happened to survive); the migration is
645+
* {@link ModelParameters#setFlashAttn}, whose tests are below.</p>
646+
*/
637647
@Test
638-
public void testEnableFlashAttn() {
648+
public void testEnableFlashAttn_deprecated_emitsNoValue() {
639649
ModelParameters p = new ModelParameters().enableFlashAttn();
640650
assertThat(p.parameters, hasKey("--flash-attn"));
641651
assertThat(p.parameters.get("--flash-attn"), is(nullValue()));
642652
}
643653

654+
@Test
655+
public void testSetFlashAttnRendersEveryMode() {
656+
// Every mode the enum declares must reach argv as its own CLI string; a mode added later is
657+
// then covered without touching this test.
658+
for (FlashAttn mode : FlashAttn.values()) {
659+
ModelParameters p = new ModelParameters().setFlashAttn(mode);
660+
assertThat(p.parameters, hasKey("--flash-attn"));
661+
assertThat(p.parameters.get("--flash-attn"), is(mode.getArgValue()));
662+
}
663+
}
664+
665+
@Test
666+
public void testSetFlashAttnPutsTheValueAfterTheKeyInArgv() {
667+
// The point of the whole change: the value must be its own argv token immediately after the
668+
// key. Anything else and llama.cpp consumes whatever follows as the mode.
669+
String[] arr = new ModelParameters().setFlashAttn(FlashAttn.ON).toArray();
670+
int key = -1;
671+
for (int i = 0; i < arr.length; i++) {
672+
if ("--flash-attn".equals(arr[i])) {
673+
key = i;
674+
break;
675+
}
676+
}
677+
assertThat("--flash-attn missing from argv", key, is(not(-1)));
678+
assertThat("--flash-attn is the last token, so its mandatory value is missing", key < arr.length - 1, is(true));
679+
assertThat(arr[key + 1], is("on"));
680+
}
681+
644682
@Test
645683
public void testEnableSwaFull() {
646684
ModelParameters p = new ModelParameters().enableSwaFull();
@@ -1092,10 +1130,13 @@ public void testToArrayComplexCombination() {
10921130
.setModel("model.gguf")
10931131
.setCtxSize(2048)
10941132
.enableEmbedding()
1095-
.enableFlashAttn();
1133+
.setFlashAttn(FlashAttn.ON);
10961134
String[] arr = p.toArray();
1097-
// argv[0]="" + --fit + on + --model + model.gguf + --ctx-size + 2048 + --embedding + --flash-attn = 9
1098-
assertThat(arr, arrayWithSize(9));
1135+
// argv[0]="" + --fit + on + --model + model.gguf + --ctx-size + 2048 + --embedding
1136+
// + --flash-attn + on = 10.
1137+
// This used to call enableFlashAttn() and assert 9, which pinned the valueless emission as
1138+
// the expected argv shape -- the very defect setFlashAttn exists to fix.
1139+
assertThat(arr, arrayWithSize(10));
10991140
assertThat(arr[0], is(""));
11001141
}
11011142

0 commit comments

Comments
 (0)