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): CLIP search integration #1939

Merged
merged 22 commits into from
Mar 18, 2023
Merged

Conversation

alextran1502
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Mar 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
immich ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 18, 2023 at 2:41AM (UTC)

}

try {
const clip = await this.machineLearning.encodeImage({ thumbnailPath: asset.resizePath });
Copy link
Member

Choose a reason for hiding this comment

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

I think we should be able to pass in a full-size image and the library will resize it appropriately, that might yield slightly better results than using an already-resized thumb?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea agreed, we should have each service do the resize itself imo so they get the exact size they need for optional recognition

server/libs/infra/src/db/entities/smart-info.entity.ts Outdated Show resolved Hide resolved
machine-learning/src/main.py Outdated Show resolved Hide resolved
Comment on lines 244 to 299
const { results } = await this.client.multiSearch.perform({
searches: [
{
collection: alias.collection_name,
q: '*',
vector_query: `smartInfo.clip:([${input.join(',')}], k:100)`,
} as any,
],
});
Copy link
Member

Choose a reason for hiding this comment

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

I think this search currently doesn't scope by userId, is that right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yeah forgot to add that back in

Copy link
Member

Choose a reason for hiding this comment

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

Also, from playing around on the preview a little: It seems like we're always returning 10 results exactly, and not bounding by a minimum score, is that correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Great question, will have to research what the default page size is here

Copy link
Contributor

Choose a reason for hiding this comment

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

Scoped by user id, an no idea what minimum distance for cutting off results, limited to 100 results though.

@bo0tzz bo0tzz changed the title feat(server): CLIP search intergration feat(server): CLIP search integration Mar 5, 2023
@bo0tzz
Copy link
Member

bo0tzz commented Mar 6, 2023

Searching currently errors out pretty hard if typesense hasn't been initialized yet, we should probably handle that a little bit better.

Error
[Nest] 1  - 03/06/2023, 9:37:43 AM   ERROR [ExceptionsHandler] Request failed with HTTP code 404 | Server said: Not Found
ObjectNotFound: Request failed with HTTP code 404 | Server said: Not Found
    at ObjectNotFound.TypesenseError [as constructor] (/usr/src/app/node_modules/typesense/lib/Typesense/Errors/TypesenseError.js:23:28)
    at new ObjectNotFound (/usr/src/app/node_modules/typesense/lib/Typesense/Errors/ObjectNotFound.js:25:42)
    at ApiCall.customErrorForResponse (/usr/src/app/node_modules/typesense/lib/Typesense/ApiCall.js:338:21)
    at /usr/src/app/node_modules/typesense/lib/Typesense/ApiCall.js:199:98
    at step (/usr/src/app/node_modules/typesense/lib/Typesense/ApiCall.js:33:23)
    at Object.next (/usr/src/app/node_modules/typesense/lib/Typesense/ApiCall.js:14:53)
    at step (/usr/src/app/node_modules/typesense/lib/Typesense/ApiCall.js:18:139)
    at Object.next (/usr/src/app/node_modules/typesense/lib/Typesense/ApiCall.js:14:53)
    at fulfilled (/usr/src/app/node_modules/typesense/lib/Typesense/ApiCall.js:5:58)
    at runMicrotasks (<anonymous>)

image

Things were reindexed by running the metadata extract job, but this error kept happening until I restarted the server container - not sure why that was.

@jrasm91 jrasm91 force-pushed the feat/clips-integration branch from bfa3511 to e03d921 Compare March 8, 2023 03:27
@jrasm91 jrasm91 force-pushed the feat/clips-integration branch from 983e12c to 042bfe1 Compare March 8, 2023 05:46
@jrasm91 jrasm91 marked this pull request as ready for review March 8, 2023 16:22
@alextran1502 alextran1502 merged commit f56eaae into main Mar 18, 2023
@alextran1502 alextran1502 deleted the feat/clips-integration branch March 18, 2023 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants