Skip to content

Commit 2cd8f26

Browse files
authored
Stop reading a backend config field that no longer exists (pytorch#22298)
## Summary The code fix this pull request opened with has since landed on main separately, so what is left here is the test that keeps it from coming back. The bug: exporting llama for Core ML failed before it started, because the branch selecting the Core ML lowering read a backend config field that no longer existed. ``` AttributeError: 'BackendConfig' object has no attribute 'mps' ``` `or` short-circuits, so this raised only when Vulkan was off. Core ML alone and Core ML with QNN raised; Core ML with Vulkan did not, because the first term was already true. ## What is in this change now Three tests, one per route through that branch, with the lowering patched so they need no macOS and no coremltools: | case | routes to | | --- | --- | | Core ML alone | the Core ML lowering | | Core ML with QNN | the combined lowering, QNN partitioner kept | | Core ML with Vulkan | the combined lowering, Vulkan partitioner kept | The second and third cases matter on their own. The exclusion clause exists to keep those two partitioners, and the Core ML lowering takes neither as an argument, so routing there would drop them in silence. Both cases assert the backend flags actually reach the shared lowering, since a call count cannot see them. The mutation matrix is complete: ``` drop the vulkan term the Vulkan case fails drop the qnn term the QNN case fails delete the whole clause both combined cases fail restore the removed field both Core ML cases fail ``` The model preparation is stubbed, because routing is decided before the model is touched. That takes the three cases from about 4.3 GB of peak memory to 420 MB. The KV cache is pinned off with a comment saying why: with it on, the source transforms import the Qualcomm SDK, which routing does not need. ## Test plan All three pass against main as it stands now, and each fails on the mutation named above.
1 parent 2b3a32d commit 2cd8f26

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

‎examples/models/llama/tests/test_export_llama_lib.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
# LICENSE file in the root directory of this source tree.
77

88
import unittest
9+
from unittest.mock import patch
910

1011
from executorch.devtools.backend_debug import get_delegation_info
1112

@@ -23,6 +24,7 @@
2324
TOSAQuantizer = None
2425
VgfQuantizer = None
2526

27+
from executorch.examples.models.llama import export_llama_lib
2628
from executorch.examples.models.llama.export_llama_lib import (
2729
_export_llama,
2830
build_args_parser,
@@ -41,6 +43,59 @@
4143

4244

4345
class ExportLlamaLibTest(unittest.TestCase):
46+
def _assert_routes_to(self, lowering, coreml=False, vulkan=False, qnn=False):
47+
"""Assert which lowering an export routes to, without running one."""
48+
llm_config = LlmConfig()
49+
llm_config.backend.coreml.enabled = coreml
50+
llm_config.backend.vulkan.enabled = vulkan
51+
llm_config.backend.qnn.enabled = qnn
52+
# _validate_args rejects dynamic shapes when Core ML or QNN is enabled.
53+
llm_config.model.enable_dynamic_shape = False
54+
# With the KV cache on, the source transforms import the Qualcomm SDK, which routing
55+
# does not need and which is not present on most machines.
56+
llm_config.model.use_kv_cache = False
57+
58+
class Reached(Exception):
59+
pass
60+
61+
# Routing is decided before the model is touched, so the export is stubbed out: without
62+
# this each case traces and lowers the whole model to check one branch.
63+
with patch.object(
64+
export_llama_lib, lowering, side_effect=Reached
65+
) as target, patch.object(export_llama_lib, "_prepare_for_llama_export"):
66+
with self.assertRaises(Reached):
67+
_export_llama(llm_config)
68+
target.assert_called_once()
69+
return target
70+
71+
def test_core_ml_alone_routes_to_the_core_ml_lowering(self):
72+
"""Core ML on its own must reach the Core ML lowering.
73+
74+
The guard read a backend config field that no longer exists, so this raised AttributeError
75+
before reaching any lowering.
76+
"""
77+
self._assert_routes_to("_to_edge_and_lower_llama_coreml", coreml=True)
78+
79+
def test_core_ml_with_qnn_routes_to_the_combined_lowering(self):
80+
"""Core ML with QNN must keep the QNN partitioner, so it takes the combined lowering."""
81+
target = self._assert_routes_to(
82+
"_to_edge_and_lower_llama", coreml=True, qnn=True
83+
)
84+
self.assertTrue(target.call_args.kwargs["coreml"])
85+
self.assertTrue(target.call_args.kwargs["qnn"])
86+
87+
def test_core_ml_with_vulkan_routes_to_the_combined_lowering(self):
88+
"""Core ML with Vulkan must keep the Vulkan partitioner the same way.
89+
90+
This is what pins the Vulkan half of the exclusion clause: the Core ML lowering takes no
91+
Vulkan argument, so routing there would drop the partitioner silently.
92+
"""
93+
target = self._assert_routes_to(
94+
"_to_edge_and_lower_llama", coreml=True, vulkan=True
95+
)
96+
self.assertTrue(target.call_args.kwargs["coreml"])
97+
self.assertTrue(target.call_args.kwargs["vulkan"])
98+
4499
def test_has_expected_ops_and_op_counts(self):
45100
"""
46101
Checks the presence of unwanted expensive ops.

0 commit comments

Comments
 (0)