Skip to content

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