[Core ML] Improve error logging #9801

cymbalrush · 2025-04-01T15:54:34Z

Summary

Fixes error logging.

The error message was getting ignored when logging to ET_LOG
If the compiled asset was corrupted then the init failed
Fixed header includes

Test plan

Unit tests
Manually deleted the cached asset and verified the init succeeded and the error message was logged.

ExecuTorch: Verifying mv3 model
I 00:00:00.119292 executorch:ETCoreMLModelManager.mm:461] Cache Hit: Successfully retrieved model with identifier=executorch_320d2b91-e091-4f92-9d4f-30b0d9eca1da_all from the models cache.
I 00:00:00.548662 executorch:main.mm:373] Load duration = 430.172000
I 00:00:00.548677 executorch:main.mm:376] Running method = forward
I 00:00:00.551965 executorch:main.mm:379] Inputs prepared.
I 00:00:00.552874 executorch:main.mm:385] Model executed successfully.
I 00:00:00.552878 executorch:main.mm:388] Inference latency=0.90ms.

pytorch-bot · 2025-04-01T15:54:38Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9801

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

⏳ No Failures, 2 Pending

As of commit 6c1f90d with merge base c178637 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot · 2025-04-01T16:48:47Z

@shoumikhin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

shoumikhin · 2025-04-01T16:48:59Z

@pytorchbot label "topic: not user facing"

shoumikhin · 2025-04-01T16:51:49Z

backends/apple/coreml/runtime/delegate/ETCoreMLAssetManager.mm

 #import "ETCoreMLAssetManager.h"
-#import <ETCoreMLAsset.h>
-#import <ETCoreMLLogging.h>
-#import <database.hpp>
+#import "ETCoreMLAsset.h"
+#import "ETCoreMLLogging.h"
+#import "database.hpp"
 #import <iostream>
-#import <json_key_value_store.hpp>
-#import <serde_json.h>
+#import "json_key_value_store.hpp"
+#import "serde_json.h"
 #import <sstream>


nit: can we group the imports by type?

#import "ETCoreMLAssetManager.h" #import "ETCoreMLAsset.h" #import "ETCoreMLLogging.h" #import "database.hpp" #import "json_key_value_store.hpp" #import "serde_json.h" #import <iostream> #import <sstream>

facebook-github-bot · 2025-04-01T20:36:39Z

@shoumikhin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

pytorch-bot · 2025-04-01T21:38:08Z

To add the ciflow label ciflow/trunk please first approve the workflows that are awaiting approval (scroll to the bottom of this page).

This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows.

metascroy · 2025-04-01T21:42:51Z

Looks great! Thanks for the PR.

I added ci/trunk to run our mac jobs in CI.

@cymbalrush do you know if the tests in backends/apple/coreml/runtime/test run in our CI? If not, are you running them locally?

pytorch-bot · 2025-04-01T21:43:03Z

To add the ciflow label ciflow/trunk please first approve the workflows that are awaiting approval (scroll to the bottom of this page).

This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows.

cymbalrush · 2025-04-01T21:45:15Z

Looks great! Thanks for the PR.

I added ci/trunk to run our mac jobs in CI.

@cymbalrush do you know if the tests in backends/apple/coreml/runtime/test run in our CI? If not, are you running them locally?

Yes they are part of the CI. I ran them locally also.

metascroy · 2025-04-01T23:59:11Z

@cymbalrush looks like there are some CI failures. Can you also run the lintrunner? (Once dev-requirements are installed, you can run lintrunner -a from executorch).

cymbalrush · 2025-04-03T16:06:51Z

@cymbalrush looks like there are some CI failures. Can you also run the lintrunner? (Once dev-requirements are installed, you can run lintrunner -a from executorch).

Ran lintrunner and fixed the failure.

metascroy · 2025-04-03T18:12:02Z

@cymbalrush looks like there are some CI failures. Can you also run the lintrunner? (Once dev-requirements are installed, you can run lintrunner -a from executorch).

Ran lintrunner and fixed the failure.

Re-running CI

facebook-github-bot · 2025-04-03T18:29:37Z

@shoumikhin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

cymbalrush · 2025-04-04T17:28:06Z

@shoumikhin @metascroy looks like build-frameworks-ios is failing when building portable_kernels

https://productionresultssa4.blob.core.windows.net/actions-results/c0d69548-fdd6-4696-a2fc-25fff2b04490/workflow-job-run-8edadf9b-7aa7-5a18-8b38-b5e145cc05f2/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-04-04T17%3A34%3A39Z&sig=h08sQhsCzNvgZjoh4VJoqEGOCS8ps7tPukfsfO7UHBM%3D&ske=2025-04-05T04%3A29%3A42Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2025-04-04T16%3A29%3A42Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-01-05&sp=r&spr=https&sr=b&st=2025-04-04T17%3A24%3A34Z&sv=2025-01-05

facebook-github-bot · 2025-04-04T17:42:09Z

@shoumikhin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

shoumikhin · 2025-04-04T17:42:30Z

@cymbalrush it doesn't seem related to your change though.

cymbalrush · 2025-04-04T19:41:26Z

@cymbalrush it doesn't seem related to your change though.

Yes, it's not related to the change.

shoumikhin · 2025-04-04T20:08:21Z

backends/apple/coreml/runtime/delegate/ETCoreMLLogging.h

+
+#if ET_LOG_ENABLED
+#define ETCoreMLLogInfo(formatString, ...) \
+    ET_LOG(Info, "%s", [NSString stringWithFormat:formatString, ##__VA_ARGS__].UTF8String)


Suggested change

ET_LOG(Info, "%s", [NSString stringWithFormat:formatString, ##__VA_ARGS__].UTF8String)

ET_LOG(Info, "%s", [NSString stringWithFormat:@formatString, ##__VA_ARGS__].UTF8String)

@cymbalrush here's the root cause

metascroy · 2025-04-04T23:02:15Z

@cymbalrush there are a lot of CI failures, e.g., in unittest / macos / macos-job:

/Users/ec2-user/runner/_work/executorch/executorch/pytorch/executorch/backends/apple/coreml/runtime/delegate/ETCoreMLModelManager.mm:528:21: error: string literal must be prefixed by '@'
2025-04-04T22:50:43.8379490Z       ETCoreMLLogInfo("Cache Miss: Model with identifier=%@ was not found in the models cache.", identifier);
2025-04-04T22:50:43.8379880Z                       ^

cymbalrush · 2025-04-08T19:56:23Z

@cymbalrush there are a lot of CI failures, e.g., in unittest / macos / macos-job:

/Users/ec2-user/runner/_work/executorch/executorch/pytorch/executorch/backends/apple/coreml/runtime/delegate/ETCoreMLModelManager.mm:528:21: error: string literal must be prefixed by '@'
2025-04-04T22:50:43.8379490Z       ETCoreMLLogInfo("Cache Miss: Model with identifier=%@ was not found in the models cache.", identifier);
2025-04-04T22:50:43.8379880Z                       ^

@shoumikhin @metascroy Sorry I had the change in my local but it was not pushed. Should be fixed now.

facebook-github-bot · 2025-04-08T19:56:40Z

@shoumikhin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

metascroy · 2025-04-08T20:09:41Z

@cymbalrush there are a lot of CI failures, e.g., in unittest / macos / macos-job:
/Users/ec2-user/runner/_work/executorch/executorch/pytorch/executorch/backends/apple/coreml/runtime/delegate/ETCoreMLModelManager.mm:528:21: error: string literal must be prefixed by '@'
2025-04-04T22:50:43.8379490Z       ETCoreMLLogInfo("Cache Miss: Model with identifier=%@ was not found in the models cache.", identifier);
2025-04-04T22:50:43.8379880Z                       ^
@shoumikhin @metascroy Sorry I had the change in my local but it was not pushed. Should be fixed now.

Re-running CI

facebook-github-bot · 2025-04-10T00:12:37Z

@metascroy has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

metascroy · 2025-04-10T04:14:25Z

@cymbalrush there is one more CI failure in building the macOS wheels.

It is a linking issue in building coreml_executor_runner. You only changed the build type from Release to Debug, but maybe in conflicts with something in the coreml/workspace?

Separately, I'm not really sure why the coreml_executor_runner is being built as part of the executorch wheels. Maybe @jathu knows why?

jathu · 2025-04-10T15:13:55Z

@metascroy it's because we build the wheels, export a model using that wheel, then test the exported model with the coreml_executor_runner. If there is a build error, we need to fix it

executorch/.ci/scripts/test_model.sh

Lines 225 to 247 in f2a08da

    
           "${PYTHON_EXECUTABLE}" -m examples.apple.coreml.scripts.export --model_name="${MODEL_NAME}" --compute_precision "${DTYPE}" 
        
           EXPORTED_MODEL=$(find "." -type f -name "${MODEL_NAME}*.pte" -print -quit) 
        
           if [ -n "$EXPORTED_MODEL" ]; then 
        
             EXPORTED_MODEL_WITH_DTYPE="${EXPORTED_MODEL%.pte}_${DTYPE}.pte" 
        
             mv "$EXPORTED_MODEL" "$EXPORTED_MODEL_WITH_DTYPE" 
        
             EXPORTED_MODEL="$EXPORTED_MODEL_WITH_DTYPE" 
        
             echo "OK exported model: $EXPORTED_MODEL" 
        
           else 
        
             echo "[error] failed to export model: no .pte file found" 
        
             exit 1 
        
           fi 
        
           # Run the model 
        
           if [ "${should_test}" = true ]; then 
        
             echo "Installing requirements needed to build coreml_executor_runner..." 
        
             backends/apple/coreml/scripts/install_requirements.sh 
        
             echo "Testing exported model with coreml_executor_runner..." 
        
             local out_dir=$(mktemp -d) 
        
             COREML_EXECUTOR_RUNNER_OUT_DIR="${out_dir}" examples/apple/coreml/scripts/build_executor_runner.sh 
        
             "${out_dir}/coreml_executor_runner" --model_path "${EXPORTED_MODEL}" 
        
           fi

Signed-off-by: Gyanendra Sinha <[email protected]> Address comments Address comments

cymbalrush · 2025-04-14T21:16:43Z

Updated to build for Release mode by default and added an option to build for Debug mode.

facebook-github-bot · 2025-04-15T00:05:41Z

@metascroy has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-04-15T00:12:21Z

@metascroy has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

metascroy · 2025-04-15T00:29:42Z

Landing from phabricator

Differential Revision: D72251859 Pull Request resolved: pytorch#9801

cymbalrush requested review from cccclai, metascroy and shoumikhin as code owners April 1, 2025 15:54

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 1, 2025

pytorch-bot bot added the topic: not user facing label Apr 1, 2025

shoumikhin reviewed Apr 1, 2025

View reviewed changes

cymbalrush force-pushed the fix_logging branch from e0ed175 to eef1802 Compare April 1, 2025 20:33

metascroy added the ciflow/trunk label Apr 1, 2025

pytorch-bot bot removed the ciflow/trunk label Apr 1, 2025

metascroy added the ciflow/trunk label Apr 1, 2025

pytorch-bot bot removed the ciflow/trunk label Apr 1, 2025

cymbalrush force-pushed the fix_logging branch 2 times, most recently from 8ad1594 to 9d8e1be Compare April 3, 2025 15:03

shoumikhin reviewed Apr 4, 2025

View reviewed changes

cymbalrush force-pushed the fix_logging branch from 8412383 to 8b9dc43 Compare April 8, 2025 19:54

metascroy added the ciflow/trunk label Apr 8, 2025

pytorch-bot bot removed the ciflow/trunk label Apr 8, 2025

github-actions bot mentioned this pull request Apr 14, 2025

Weekly pr metrics report - 2025-04-01..2025-04-07 wdvr/pytorch#28

Open

Improve error logging

3366aa1

Signed-off-by: Gyanendra Sinha <[email protected]> Address comments Address comments

cymbalrush force-pushed the fix_logging branch from c28053b to 3366aa1 Compare April 14, 2025 21:15

metascroy added 2 commits April 14, 2025 15:02

Merge branch 'main' into fix_logging

14c252b

Merge branch 'main' into fix_logging

6c1f90d

metascroy approved these changes Apr 15, 2025

View reviewed changes

facebook-github-bot merged commit 1bf3cf5 into pytorch:main Apr 15, 2025
97 checks passed

github-actions bot mentioned this pull request Apr 21, 2025

Weekly pr metrics report - 2025-04-01..2025-04-07 wdvr/pytorch#30

Open

keyprocedure pushed a commit to keyprocedure/executorch that referenced this pull request Apr 21, 2025

[Core ML] Improve error logging

c956664

Differential Revision: D72251859 Pull Request resolved: pytorch#9801

github-actions bot mentioned this pull request May 5, 2025

Weekly pr metrics report - 2025-04-01..2025-04-07 wdvr/pytorch#35

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core ML] Improve error logging #9801

[Core ML] Improve error logging #9801

cymbalrush commented Apr 1, 2025

pytorch-bot bot commented Apr 1, 2025 •

edited

Loading

facebook-github-bot commented Apr 1, 2025

shoumikhin commented Apr 1, 2025

shoumikhin Apr 1, 2025

cymbalrush Apr 1, 2025

facebook-github-bot commented Apr 1, 2025

pytorch-bot bot commented Apr 1, 2025

metascroy commented Apr 1, 2025

pytorch-bot bot commented Apr 1, 2025

cymbalrush commented Apr 1, 2025

metascroy commented Apr 1, 2025

cymbalrush commented Apr 3, 2025

metascroy commented Apr 3, 2025

facebook-github-bot commented Apr 3, 2025

cymbalrush commented Apr 4, 2025

facebook-github-bot commented Apr 4, 2025

shoumikhin commented Apr 4, 2025

cymbalrush commented Apr 4, 2025

shoumikhin Apr 4, 2025

shoumikhin Apr 4, 2025 •

edited

Loading

metascroy commented Apr 4, 2025

cymbalrush commented Apr 8, 2025

facebook-github-bot commented Apr 8, 2025

metascroy commented Apr 8, 2025

facebook-github-bot commented Apr 10, 2025

metascroy commented Apr 10, 2025

jathu commented Apr 10, 2025

cymbalrush commented Apr 14, 2025 •

edited

Loading

facebook-github-bot commented Apr 15, 2025

facebook-github-bot commented Apr 15, 2025

metascroy commented Apr 15, 2025

	ET_LOG(Info, "%s", [NSString stringWithFormat:formatString, ##__VA_ARGS__].UTF8String)
	ET_LOG(Info, "%s", [NSString stringWithFormat:@formatString, ##__VA_ARGS__].UTF8String)

[Core ML] Improve error logging #9801

[Core ML] Improve error logging #9801

Conversation

cymbalrush commented Apr 1, 2025

Summary

Test plan

pytorch-bot bot commented Apr 1, 2025 • edited Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9801

⏳ No Failures, 2 Pending

facebook-github-bot commented Apr 1, 2025

shoumikhin commented Apr 1, 2025

shoumikhin Apr 1, 2025

Choose a reason for hiding this comment

cymbalrush Apr 1, 2025

Choose a reason for hiding this comment

facebook-github-bot commented Apr 1, 2025

pytorch-bot bot commented Apr 1, 2025

metascroy commented Apr 1, 2025

pytorch-bot bot commented Apr 1, 2025

cymbalrush commented Apr 1, 2025

metascroy commented Apr 1, 2025

cymbalrush commented Apr 3, 2025

metascroy commented Apr 3, 2025

facebook-github-bot commented Apr 3, 2025

cymbalrush commented Apr 4, 2025

facebook-github-bot commented Apr 4, 2025

shoumikhin commented Apr 4, 2025

cymbalrush commented Apr 4, 2025

shoumikhin Apr 4, 2025

Choose a reason for hiding this comment

shoumikhin Apr 4, 2025 • edited Loading

Choose a reason for hiding this comment

metascroy commented Apr 4, 2025

cymbalrush commented Apr 8, 2025

facebook-github-bot commented Apr 8, 2025

metascroy commented Apr 8, 2025

facebook-github-bot commented Apr 10, 2025

metascroy commented Apr 10, 2025

jathu commented Apr 10, 2025

cymbalrush commented Apr 14, 2025 • edited Loading

facebook-github-bot commented Apr 15, 2025

facebook-github-bot commented Apr 15, 2025

metascroy commented Apr 15, 2025

pytorch-bot bot commented Apr 1, 2025 •

edited

Loading

shoumikhin Apr 4, 2025 •

edited

Loading

cymbalrush commented Apr 14, 2025 •

edited

Loading