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

feat(ml): improve test coverage #7041

Merged
merged 7 commits into from Feb 11, 2024
Merged

feat(ml): improve test coverage #7041

merged 7 commits into from Feb 11, 2024

Conversation

mertalev
Copy link
Contributor

@mertalev mertalev commented Feb 11, 2024

Description

This PR adds a bunch of tests, increasing unit test coverage from 80% to 84%, and increasing coverage with E2E tests included from 89% to 92%.

Other changes:

  • Updates the E2E tests that were failing
    • In the case of the CLIP models, this was because we now normalize the embeddings from those models so they didn't match with the non-normalized embeddings
    • For facial recognition, the preprocessing is now done with NumPy so the numbers are very slightly different.
    • These tests have been updated to use allclose for comparison to avoid floating point flakiness, with an absolute tolerance of 0.00000001.
  • Tests no longer hardcode /cache and respect the MACHINE_LEARNING_MODEL_CACHE env for assertions
  • Update test workflow to print lines with missing coverage
  • Remove dead code that either only existed for reasons that are no longer relevant, or that were never used at all and only existed for the sake of completeness

Copy link

cloudflare-pages bot commented Feb 11, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3fd388d
Status: ✅  Deploy successful!
Preview URL: https://9f704191.immich.pages.dev
Branch Preview URL: https://feat-ml-test-coverage.immich.pages.dev

View logs

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Lgtm

@mertalev mertalev merged commit 0c4df21 into main Feb 11, 2024
24 checks passed
@mertalev mertalev deleted the feat/ml-test-coverage branch February 11, 2024 22:58
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.

None yet

2 participants