Skip to content

[CoreML] Add support for running statefule model. #5143

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
Sep 7, 2024

Conversation

cymbalrush
Copy link
Contributor

Add support for running stateful model. On iOS18 CoreML has a new API predictionFromFeatures:usingState:options:error that supports model state. The runtime change makes sure to call the new API if the model has state otherwise it uses the old API.

Testing:
Did local testing. To generate the test model I am waiting on the coremltools change to be publicly available.

Copy link

pytorch-bot bot commented Sep 6, 2024

🔗 Helpful Links

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

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

❌ 3 New Failures, 1 Unrelated Failure

As of commit aef0bc5 with merge base 9739609 (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@facebook-github-bot 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 Sep 6, 2024
@cccclai
Copy link
Contributor

cccclai commented Sep 6, 2024

Thanks! How do we test it?

@cymbalrush
Copy link
Contributor Author

Thanks! How do we test it?

I will add a unit test but have to wait for @YifanShenSZ coremltools changes to be available.

@cccclai
Copy link
Contributor

cccclai commented Sep 6, 2024

Thanks! How do we test it?

I will add a unit test but have to wait for @YifanShenSZ coremltools changes to be available.

Great! Looks like the coreml related tests are failing. What's your preference here? Should we want for coremltools update before merging this change?

@cymbalrush
Copy link
Contributor Author

cymbalrush commented Sep 6, 2024

Thanks! How do we test it?

I will add a unit test but have to wait for @YifanShenSZ coremltools changes to be available.

Great! Looks like the coreml related tests are failing. What's your preference here? Should we want for coremltools update before merging this change?

I am fixing the tests, it's API availability issue. The change shouldn't break anything.

@facebook-github-bot
Copy link
Contributor

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

@cccclai
Copy link
Contributor

cccclai commented Sep 7, 2024

Looks great. Thank you

@cccclai cccclai merged commit 13da62b into pytorch:main Sep 7, 2024
97 of 102 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants