Skip to content

Conversation

@QuentinLemCode
Copy link

@QuentinLemCode QuentinLemCode commented Jan 23, 2025

This PR take the work from @ghisch and try to make it production ready

#1218

@QuentinLemCode QuentinLemCode changed the title Switch human Switch to @vladmandic/human Jan 23, 2025
@QuentinLemCode QuentinLemCode marked this pull request as draft January 23, 2025 22:41
@Felitendo
Copy link

Nothing productive to add, just a thank you that this is finallly being worked on <3

@marcelklehr marcelklehr marked this pull request as ready for review January 24, 2025 14:18
@marcelklehr
Copy link
Member

Mmh, not sure why we don't have CI here

@bonswouar
Copy link
Collaborator

I'm really not sure why, but I had the option to click on "Approve and run" [the CI] so I just did and it worked 😅

@marcelklehr
Copy link
Member

@bonswouar You are a collaborator because you contributed previously ;)

@github-actions
Copy link

github-actions bot commented Feb 7, 2025

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@QuentinLemCode
Copy link
Author

I have to update my branch with the latest change
Also I haven't achieved to launch PHPUnit tests locally

ghisch and others added 17 commits February 9, 2025 23:02
Signed-off-by: ghisch <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Adapt dimensions to 1024 as outputed by @vladmandic/human

Signed-off-by: ghisch <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: ghisch <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: ghisch <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>

# Conflicts:
#	vendor-bin/php-scoper/composer.lock
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Joas Schilling <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
nextcloud-bot and others added 2 commits February 9, 2025 23:02
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
Signed-off-by: QuentinLemCode <[email protected]>
@MB-Finski
Copy link
Contributor

MB-Finski commented Feb 20, 2025

I've just had time to take a quick peek but here's how you can currently get some tentative clustering results:

  • Set max edge length to some large value, like 99, for now
  • Hardcode minSampleSize to 2 in the FaceClusterAnalyzer.php

In the test dataset that I'm using (3651 images of 100 largest identities from the LFW dataset) the final clustering results are still inferior to the current main branch but that's not surprising given that the hyper parameters have been optimized for the old detection/embedding models.

Currently the old models achieve:

  • 3023 detected faces
  • 2904 clustered faces

The new models achieve:

  • 4013 detected faces (some images contain additional faces in the background)
  • 2717 clustered faces

I didn't yet have time to write a quality check routine for the clusters but after checking some of the clusters by hand I didn't notice any erroneous identity assignments or "shit clusters" for either new or old model combos. In fact, the clustering results by the old models are pretty amazing considering that the face detection model misses quite a few faces.

Next I'll have to write an optimization script to narrow in on better hyper parameters. I'll get back to you!

EDIT: Oh, and patch this pr in if you want to test clustering more extensively; it accelerates the clustering by nearly 2x.

@MB-Finski
Copy link
Contributor

Hmm.. Although the face embeddings seem to have 1024 dimensions in the db, the error logs are getting spammed with: "More than 1000 expressions in a list are not allowed on Oracle.". It might be that the embeddings will have to be split into two if 1024 dimensional embeddings are used.

@marcelklehr
Copy link
Member

More than 1000 expressions in a list are not allowed on Oracle.

This is likely unrelated to the embedding dimensions, I think

@MB-Finski
Copy link
Contributor

Ok! The error seems to result from insertion of the new face detections to the db. But they still have the full 1024 dimensions regardless, so I guess we can just ignore that for now.

Anyhow: I've scoped some reasonable ranges for the hyper-parameters and I cannot get the default HSE FaceRes embedding model to outperform the current FaceNet model. Either there's something wrong with the pipeline in either the @vladmandic/human -end or in the way we implement it OR the HSE FaceRes embedding model just isn't very good. I wouldn't be too surprised if the latter was true. The HSE FaceRes model is somewhat sketchy to say the least. It's really difficult to find too much information about it or even some reliable benchmarks. In any case, having 1024 dimensional embeddings is not necessarily a good idea for clustering...

I also did some testing with alternate distance metrics; the HSE FaceRes model seems to perform better when using Minkowski distance with p-value set to something rather high like 4 or greater. This provides more circumstantial evidence for the fact that the resulting 1024 dimensional embeddings are "information sparse": i.e. only some of the dimensions seem to hold meaningful information.

While I'm at it, I'll do some testing with the InsightFace models that vladmandic has converted over to tfjs but I'm not holding my breath.

@MB-Finski
Copy link
Contributor

Soooo, finally after running a lot of tests I got HSE FaceRes to beat the current FaceNet results (3109 vs 2904 clustered faces), quality metrics for established clusters are similar for both. The secret sauce seems to be to use "sparse cosine" as the distance measure. This is most curious since the default distance measure used for HSE FaceRes in @vladmandic/human is Euclidean distance. These kinds of omissions make me wonder how well other parts of the embedding pipeline are implemented in @vladmandic/human. For example, to get the best results with any face embedding model it's usually mandatory to use the same face detection model that was used for preprocessing the training images. I wonder if this is the case for @vladmandic/human since there are no end-to-end benchmark results available.

However, the issue with sparse cosine distance is that it's really slow to compute: it's something like 3-4 times slower than the current squared distance. That's a quite significant drawback considering that the improvement over FaceNet is somewhat marginal. All this makes me wonder if it would be just better to train/fine-tune an embedding model specifically for Recognize..? We could use the FaceNet model as base and gather training data on situations where we know the model doesn't perform well (side profile pictures, pictures of children etc). I think this would be the way that we could get a significant leap in clustering performance. WDYT?

@marcelklehr
Copy link
Member

All this makes me wonder if it would be just better to train/fine-tune an embedding model specifically for Recognize..?

I'm not sure if this is worth the effort. Nextcloud Staff is planned to create an ExApp version of recognize in python soon which will hopefully be SOTA as well as easier to maintain. I'd be happy if we could have your expertise in the brainstorms if you'd be available @MB-Finski :)

@MB-Finski
Copy link
Contributor

@QuentinLemCode I opened a pr at your fork with the changes necessary for better clustering performance with @vladmandic/human.

@marcelklehr I'd be more than eager to participate! Ping me when it's happening. I'm usually best able to participate Mon and Tue.

@marcelklehr
Copy link
Member

Hi @QuentinLemCode

Thank you for your contribution to recognize. I appreciate your effort and the work you've put into it and I'd like to invite you to our Developer Community Chat. Hit me up if you would like a guest account for that instance!

However, we wanted to inform you that we are planning to port this application to Python. Due to this significant change, the modifications proposed in your pull request will no longer be relevant, and we have decided not to pursue this PR further due to the high effort required for the hyper parameter tuning.

We hope you understand our decision, and we encourage you to contribute to the project once we start working on the Python version.

Thank you for your understanding and continued support.

Best regards,
The Nextcloud Team

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.

8 participants