Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

ArcFace Loss Implementation #291

Open
wants to merge 30 commits into
base: development
Choose a base branch
from

Conversation

aylinaydincs
Copy link

  • This pull_request is a part of Google Summer of Code Project mentored bt @monatis at TensorFlow Organization.
  • In this pull-request, I implement ArcFace Loss, and I also provide a sample notebook to show how to use it standalone and model.compile()
  • I wrote a introductory medium story and a more comprehensive and advanced one will be coming soon
  • I wrote unit tests for serializaiton and loss calculation

Ps. Previous Pull Request Deleted, due to the conflict of commits

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@owenvallis owenvallis self-requested a review September 22, 2022 06:03
@owenvallis owenvallis self-assigned this Sep 22, 2022
@owenvallis owenvallis added the component:losses Issues related to support additional metric learning technqiues (e.g loss) label Sep 22, 2022
@owenvallis
Copy link
Contributor

Thanks for the PR. Looks like there are duplicate files again for the loss and the tests, and I think you'll need to run the Black and Isort formatting (the imports in the init.py look out of order again).

Also let me know your thoughts regarding adding the sub-centers layer. I think the weights that map the embedding to the N classes (and optionally the K sub-centers) needs to be a Dense layer so that we can train the weights. The current implementation has the matrix as a variable in loss so I don't think the random values will be updated during training.

@aylinaydincs aylinaydincs changed the base branch from master to development October 6, 2022 18:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component:losses Issues related to support additional metric learning technqiues (e.g loss)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants