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(server): separate face search relation #10371

Merged
merged 7 commits into from
Jun 16, 2024

Conversation

mertalev
Copy link
Contributor

@mertalev mertalev commented Jun 15, 2024

Description

This PR addresses a subtle issue with the current facial recognition. Each time a face is assigned or reassigned a person, for both the initial and later facial recognition runs, an additional duplicate embedding is inserted into the face vector index. This can lead to index degradation as the majority of the index is duplicated, in turn leading to faces sometimes not being recognized when they should.

This PR changes the embedding to be in a separate table as a one-one relation, similar to how smart search is handled. This means changes to the face, such as which person it's assigned to, have no effect on the index. It has a smaller but notable benefit of making these changes faster and producing less WAL.

A notable benefit of this change is also that it makes supporting manually added faces easier as an embedding is no longer required.

Also sets storage to external so Postgres doesn't try to compress the embeddings, following the finding here.

Fixes #10277

How Has This Been Tested?

Tested that the migration is successful without loss of data and that both face detection and facial recognition jobs continue to work.

  • The number of unassigned faces decreased by 38% after running with this change
  • Facial recognition is twice as fast as before
  • Face detection is 20% faster (I'm not really sure why)
  • SELECT idx_tuples FROM pg_vector_index_stat WHERE indexname = 'face_index'; is identical to the number of faces, i.e. no duplicate embeddings

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.

Can we also mark the clip embeddings as external?

server/src/entities/face-search.entity.ts Show resolved Hide resolved
@mertalev
Copy link
Contributor Author

Can we also mark the clip embeddings as external?

Yep, this makes the clip embeddings external too.

@mertalev mertalev force-pushed the chore/separate-face-search-relation branch from 1540f20 to 2adf3cf Compare June 16, 2024 19:20
@mertalev mertalev enabled auto-merge (squash) June 16, 2024 19:21
@mertalev mertalev merged commit 6b1b505 into main Jun 16, 2024
22 checks passed
@mertalev mertalev deleted the chore/separate-face-search-relation branch June 16, 2024 19:25

await queryRunner.query(`ALTER TABLE asset_faces ADD COLUMN "embedding" vector(512)`);
await queryRunner.query(`ALTER TABLE face_search ALTER COLUMN embedding SET STORAGE DEFAULT`);
await queryRunner.query(`ALTER TABLE smart_search ALTER COLUMN embedding SET STORAGE DEFAULT`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I missed this line. Nice!

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.

Unassigned faces start showing up later
2 participants