Skip to content
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

fix(ml): armnn not being used #10929

Merged
merged 2 commits into from
Jul 10, 2024
Merged

fix(ml): armnn not being used #10929

merged 2 commits into from
Jul 10, 2024

Conversation

mertalev
Copy link
Contributor

@mertalev mertalev commented Jul 6, 2024

Description

We check if an ARM NN model exists and fall back to ONNX if it does not. However, we do this check before the model is downloaded at all, so it will always appear missing unless it has already been downloaded. Additionally, since we have already fallen back to ONNX when downloading, we avoid downloading the ARM NN model. This catch-22 prevents ARM NN from being used in practice without manually moving the model into the model cache.

This PR fixes this behavior and additionally moves the fallback handling outside of the model class itself to being in the main load function. This makes the model's behavior more predictable: it's unexpected for the model to choose a different format than what the caller specified.

@mertalev mertalev requested a review from fyfrey July 6, 2024 17:09
Copy link
Contributor

@fyfrey fyfrey left a comment

Choose a reason for hiding this comment

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

Wonderful!
I've tested this on my RK3588 device and ML with ARMNN now works out-of-the-box for CLIP vision model :-)

@alextran1502 alextran1502 merged commit f43721e into main Jul 10, 2024
22 checks passed
@alextran1502 alextran1502 deleted the fix/ml-armnn-not-used branch July 10, 2024 14:20
claabs pushed a commit to claabs/immich-machine-learning-openvino-kernel-fix that referenced this pull request Aug 11, 2024
* fix armnn not being used, move fallback handling to main, add tests

* formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants