Skip to content

Fix compiled model export. #6568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion backends/apple/coreml/compiler/coreml_preprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,12 +425,15 @@ def preprocess(
CoreMLBackend.op_linear_quantizer_config_from_compile_specs(compile_specs)
)

# Load the model if MODEL_TYPE is 'COMPILED_MODEL'. This step is necessary because
# get_compiled_model_path() requires a loaded model.
skip_model_load = model_type != CoreMLBackend.MODEL_TYPE.COMPILED_MODEL
mlmodel = ct.convert(
model=edge_program,
source="pytorch",
convert_to="mlprogram",
pass_pipeline=ct.PassPipeline.DEFAULT,
skip_model_load=True,
skip_model_load=skip_model_load,
compute_precision=model_compute_precision,
minimum_deployment_target=minimum_deployment_target,
compute_units=compute_units,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class BackendDelegate {
// Max models cache size in bytes.
size_t max_models_cache_size = 10 * size_t(1024) * size_t(1024) * size_t(1024);
// If set to `true`, delegate pre-warms the most recently used asset.
bool should_prewarm_asset = true;
bool should_prewarm_asset = false;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this change @cymbalrush?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature was pre-warming the most recently used assets, which could impact the delegate initialization time and the first model load time. This optimization was beneficial only for the CPU backend and only when the subsequent asset load remained unchanged. So, disabling it by default.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cymbalrush, it's worth documenting the above behavior (on Line 30?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me create another PR for this.

// If set to `true`, delegate pre-warms the model in `init`.
bool should_prewarm_model = true;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<plist version="1.0">
<dict>
<key>shouldPrewarmAsset</key>
<true/>
<false/>
<key>shouldPrewarmModel</key>
<true/>
<key>maxAssetsSizeInBytes</key>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,12 @@ - (void)testStateProgramExecute {
}
#endif

- (void)testAddMulCompiledProgramExecute {
NSURL *modelURL = [[self class] bundledResourceWithName:@"add_mul_compiled_coreml_all" extension:@"pte"];
XCTAssertNotNil(modelURL);
[self executeModelAtURL:modelURL nLoads:1 nExecutions:2];
}

- (void)executeMultipleModelsConcurrently:(NSArray<NSURL *> *)modelURLs
nLoads:(NSUInteger)nLoads
nExecutions:(NSUInteger)nExecutions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

/* Begin PBXBuildFile section */
8307EB8A2C9262060011AE6D /* state_coreml_all.pte in Resources */ = {isa = PBXBuildFile; fileRef = 8307EB892C9262060011AE6D /* state_coreml_all.pte */; };
838CA6872CD1965700462190 /* add_mul_compiled_coreml_all.pte in Resources */ = {isa = PBXBuildFile; fileRef = 838CA6862CD1965700462190 /* add_mul_compiled_coreml_all.pte */; };
83BB78A02C65DA7300274ED7 /* ETCoreMLModelDebugInfo.mm in Sources */ = {isa = PBXBuildFile; fileRef = 83BB789F2C65DA7300274ED7 /* ETCoreMLModelDebugInfo.mm */; };
83BB78BF2C66AAAE00274ED7 /* add_mul_coreml_all.bin in Resources */ = {isa = PBXBuildFile; fileRef = 83BB78BD2C66AAAE00274ED7 /* add_mul_coreml_all.bin */; };
83BB78C02C66AAAE00274ED7 /* add_mul_coreml_all.pte in Resources */ = {isa = PBXBuildFile; fileRef = 83BB78BE2C66AAAE00274ED7 /* add_mul_coreml_all.pte */; };
Expand Down Expand Up @@ -122,6 +123,7 @@

/* Begin PBXFileReference section */
8307EB892C9262060011AE6D /* state_coreml_all.pte */ = {isa = PBXFileReference; lastKnownFileType = file; name = state_coreml_all.pte; path = ../test/models/state_coreml_all.pte; sourceTree = "<group>"; };
838CA6862CD1965700462190 /* add_mul_compiled_coreml_all.pte */ = {isa = PBXFileReference; lastKnownFileType = file; name = add_mul_compiled_coreml_all.pte; path = ../test/models/add_mul_compiled_coreml_all.pte; sourceTree = "<group>"; };
83BB789E2C65DA7300274ED7 /* ETCoreMLModelDebugInfo.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ETCoreMLModelDebugInfo.h; path = ../sdk/ETCoreMLModelDebugInfo.h; sourceTree = "<group>"; };
83BB789F2C65DA7300274ED7 /* ETCoreMLModelDebugInfo.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = ETCoreMLModelDebugInfo.mm; path = ../sdk/ETCoreMLModelDebugInfo.mm; sourceTree = "<group>"; };
83BB78BD2C66AAAE00274ED7 /* add_mul_coreml_all.bin */ = {isa = PBXFileReference; lastKnownFileType = archive.macbinary; name = add_mul_coreml_all.bin; path = ../test/models/add_mul_coreml_all.bin; sourceTree = "<group>"; };
Expand Down Expand Up @@ -606,6 +608,7 @@
C98551992AD2542D009143F9 /* mul_coreml_all.bin */,
C985519C2AD2542D009143F9 /* mul_coreml_all.pte */,
C985519B2AD2542D009143F9 /* mv3_coreml_all.bin */,
838CA6862CD1965700462190 /* add_mul_compiled_coreml_all.pte */,
C98551982AD2542D009143F9 /* mv3_coreml_all.pte */,
83BB78BD2C66AAAE00274ED7 /* add_mul_coreml_all.bin */,
83BB78BE2C66AAAE00274ED7 /* add_mul_coreml_all.pte */,
Expand Down Expand Up @@ -680,6 +683,7 @@
C985519E2AD2542D009143F9 /* mv3_coreml_all.pte in Resources */,
C98551A02AD2542D009143F9 /* add_coreml_all.bin in Resources */,
C98551A22AD2542D009143F9 /* mul_coreml_all.pte in Resources */,
838CA6872CD1965700462190 /* add_mul_compiled_coreml_all.pte in Resources */,
8307EB8A2C9262060011AE6D /* state_coreml_all.pte in Resources */,
C98551A32AD2542D009143F9 /* add_coreml_all.pte in Resources */,
);
Expand Down
9 changes: 9 additions & 0 deletions backends/apple/coreml/scripts/generate_test_models.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@ done

echo "Executorch: Generating stateful model"
python3 "$SCRIPT_DIR_PATH/../runtime/test/export_stateful_model.py"

COMPILE_MODELS=("add_mul")
echo "Executorch: Generating compiled model"
for MODEL in "${COMPILE_MODELS[@]}"
do
echo "Executorch: Generating compiled $MODEL model"
python3 -m examples.apple.coreml.scripts.export --model_name "$MODEL" --compile
mv -f "$MODEL""_compiled_coreml_all.pte" "$COREML_DIR_PATH/runtime/test/models"
done
3 changes: 2 additions & 1 deletion examples/apple/coreml/scripts/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ def main():
example_inputs,
)

save_executorch_program(exec_program, args.model_name, args.compute_unit)
model_name = f"{args.model_name}_compiled" if args.compile else args.model_name
save_executorch_program(exec_program, model_name, args.compute_unit)
generate_etrecord(f"{args.model_name}_coreml_etrecord.bin", edge_copy, exec_program)

if args.save_processed_bytes and lowered_module is not None:
Expand Down
Loading