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

Implement adaptive thresholding #31

Merged
merged 12 commits into from
May 3, 2022

Conversation

novialriptide
Copy link
Member

Fix: #19

@novialriptide novialriptide marked this pull request as draft March 9, 2022 16:07
@novialriptide novialriptide changed the title Adaptive threshold Implement adaptive thresholding Mar 9, 2022
@ryspark
Copy link
Member

ryspark commented Mar 12, 2022

Hey Andrew, thanks for working on this PR. Couple of requests before finishing this mini-project up:

Code organization

  1. compute_threshold method in facenet.py seems to just compute the dot product similarity between two vectors. Instead of defining a new method within the FaceNet class, it's best to use the existing DistMetric (from util/distance.py). Dot product distance is already implemented there in DistMetric.distance, maybe you could make an auxiliary method similarity that just returns 1 - self.distance().

Performance

  1. find_threshold creates a copy of self.data. Is it possible to rewrite that method without doing so? Not a big deal since it seems to only be called at init.
  2. apply_thresholds - if possible, can it be written without the nested for loop (ie vectorized)? If not, no big deal.
  3. find_similar_embedding - this should definitely be rewritten to avoid looping over self.data on each call. Better way to do this would be to use the SVM/KNN classifier (self.classifier.predict) to get the most similar embedding.

added proper documentation, black formatting, and codecleanup
@novialriptide
Copy link
Member Author

I resolved Code Organization: 1 and Performance: 3, I'm now working on the low-priority ones.

This PR adds some new things other than adaptive thresholding:

  • Removed No face detected spam.
  • Added support for Python3.8.10.

@novialriptide novialriptide marked this pull request as ready for review March 15, 2022 17:07
@novialriptide novialriptide requested a review from ryspark May 2, 2022 07:09
@novialriptide
Copy link
Member Author

It looks like tests are failing, but in files unrelated to this pull request. I would like to create another pull request fixing these tests. @orangese.

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.

Implement new facial detection/recognition engine
2 participants