Skip to content

Commit 88f0c74

Browse files
authored
Treat an empty LLM data path as absent (pytorch#21835)
`LlmModuleConfig.Builder` defaults `dataPath` to `""`, and `create_text_llm_runner` takes whatever the optional holds — the empty string included. So an Android caller that never asked for a separate data file still reaches the loader with `""`: ``` [llm_runner_helper.cpp:255] Reading metadata from model [mmap_data_loader.cpp:94] Failed to open : No such file or directory (2) [llm_runner_helper.cpp:105] Failed reading method names [llm_runner_helper.cpp:258] Failed to get metadata from model ``` Because `LlmModule.load()` reports failure through `throwExecutorchException`, this comes out as `Fatal signal 6 (SIGABRT)` with abort message `'ptr'` and the process dies, rather than an error the app can catch. That is what makes it hard to place: nothing in the message points at a data path, and the same `.pte` and tokenizer work everywhere else. Reproducing it takes only the documented builder call — no data file, no unusual model: ```kotlin LlmModule( LlmModuleConfig.create() .modulePath(model) .tokenizerPath(tokenizer) .modelType(LlmModuleConfig.MODEL_TYPE_TEXT) .build()) .load() ``` Passing `.dataPath(null)` explicitly is the workaround. ## The change The builder default becomes `null`, and both runner factories ignore an empty path so a caller in any language whose "no value" is an empty string lands on the no-data-file branch rather than a failed mmap. Three files: the Kotlin default, the two `data_path.has_value()` checks, and a test for the default. I kept the guard in C++ as well as fixing the Kotlin default, because the optional is public API and an empty string will keep arriving from bindings that have no null. ## Verification On a Pixel 8a, arm64, LFM2.5-350M exported to XNNPACK 8da4w: - before: abort during `load()`, every time - after: loads in ~2.0 s and generates, prefill 23.8 tok/s / decode 11.9 tok/s Those rates are low because the phone was locked while measuring, which parks the app on the little cluster — not a property of this change. I also confirmed the failure is not the model or the tokenizer: the `.pte` exposes the full metadata method set the runner expects, and the tokenizer loads and encodes identically on the same device through a standalone harness. This PR was authored with Claude.
1 parent 33f68d1 commit 88f0c74

3 files changed

Lines changed: 56 additions & 3 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
package org.pytorch.executorch
9+
10+
import androidx.test.ext.junit.runners.AndroidJUnit4
11+
import org.junit.Assert.assertEquals
12+
import org.junit.Assert.assertNull
13+
import org.junit.Test
14+
import org.junit.runner.RunWith
15+
import org.pytorch.executorch.extension.llm.LlmModuleConfig
16+
17+
/** Tests for [LlmModuleConfig]. */
18+
@RunWith(AndroidJUnit4::class)
19+
class LlmModuleConfigTest {
20+
21+
@Test
22+
fun testDataPathDefaultsToNull() {
23+
// An empty default reaches the runner as a real path, which then fails to open and takes the
24+
// whole load down. Absent has to be null.
25+
val config =
26+
LlmModuleConfig.create().modulePath("/model.pte").tokenizerPath("/tokenizer.json").build()
27+
assertNull(config.dataPath)
28+
}
29+
30+
@Test
31+
fun testDataPathRoundTrips() {
32+
val config =
33+
LlmModuleConfig.create()
34+
.modulePath("/model.pte")
35+
.tokenizerPath("/tokenizer.json")
36+
.dataPath("/weights.ptd")
37+
.build()
38+
assertEquals("/weights.ptd", config.dataPath)
39+
}
40+
41+
@Test
42+
fun testDefaults() {
43+
val config =
44+
LlmModuleConfig.create().modulePath("/model.pte").tokenizerPath("/tokenizer.json").build()
45+
assertEquals("/model.pte", config.modulePath)
46+
assertEquals("/tokenizer.json", config.tokenizerPath)
47+
assertEquals(LlmModuleConfig.MODEL_TYPE_TEXT, config.modelType)
48+
assertEquals(LlmModuleConfig.LOAD_MODE_MMAP, config.loadMode)
49+
}
50+
}

‎extension/android/executorch_android/src/main/java/org/pytorch/executorch/extension/llm/LlmModuleConfig.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private constructor(
6565
private var modulePath: String? = null
6666
private var tokenizerPath: String? = null
6767
private var temperature: Float = 0.8f
68-
private var dataPath: String? = ""
68+
private var dataPath: String? = null
6969
private var modelType: Int = MODEL_TYPE_TEXT
7070
private var numBos: Int = 0
7171
private var numEos: Int = 0

‎extension/llm/runner/llm_runner_helper.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,10 @@ std::unique_ptr<TextLLMRunner> create_text_llm_runner(
189189
float temperature,
190190
const std::string& method_name,
191191
Module::LoadMode load_mode) {
192-
if (data_path.has_value()) {
192+
// An empty path is not a path. Callers that build the optional from a
193+
// language whose "no value" is an empty string would otherwise reach the
194+
// loader with "", which fails to open and takes the whole load down with it.
195+
if (data_path.has_value() && !data_path.value().empty()) {
193196
std::vector<std::string> data_files;
194197
data_files.push_back(data_path.value());
195198
return create_text_llm_runner(
@@ -355,7 +358,7 @@ std::unique_ptr<MultimodalRunner> create_multimodal_runner(
355358

356359
// Create the Module
357360
std::unique_ptr<Module> module;
358-
if (data_path.has_value()) {
361+
if (data_path.has_value() && !data_path.value().empty()) {
359362
module = std::make_unique<Module>(model_path, data_path.value(), load_mode);
360363
} else {
361364
module = std::make_unique<Module>(model_path, load_mode);

0 commit comments

Comments
 (0)