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

[GSK-2247] Add unit tests for nme implementation #6

Merged
merged 34 commits into from
Dec 22, 2023

Conversation

Inokinoki
Copy link
Member

Compute the ME, d for normalization and NME from scratch and compare the results, using the first image of 300W indoor.

Copy link

linear bot commented Dec 8, 2023

GSK-2247 Benchmark the NME implementation

Implement a unit-test that benchmark the NME implementation against one of the numbers given in https://paperswithcode.com/sota/facial-landmark-detection-on-300w. Ideally if we can find someone the NME calculated only for one of the images contained in the 300W dataset (instead of benchmarking against the total NME of all images).

For reference for the NME definition, see section 3.1 of : https://arxiv.org/pdf/2210.07233v1.pdf

Copy link
Contributor

@rabah-khalek rabah-khalek left a comment

Choose a reason for hiding this comment

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

  • For traceability and generalisability, could we use the 2D face_alignment model on one of the benchmark images from the 300W to generate TEST_MARKS and PREDICT_MARKS?
  • I resolved the conficts due to last refactoring of tests but could you please also improve the coverage over the new validations we have for the metrics?
  • If you think benchmarking with a NME number from the literature is tedious, we can drop it, and just rely on the fact we're testing the euclidean distances using different methods.

@rabah-khalek rabah-khalek marked this pull request as draft December 9, 2023 16:12
@Inokinoki
Copy link
Member Author

Thanks for the review @rabah-khalek !

  • For traceability and generalisability, could we use the 2D face_alignment model on one of the benchmark images from the 300W to generate TEST_MARKS and PREDICT_MARKS?

Yes, I will try to add the fixture for the model and a test dataset with one image. There is already the resources on my side. They are ignored by my local git repo somehow:

image
  • I resolved the conficts due to last refactoring of tests but could you please also improve the coverage over the new validations we have for the metrics?

Doing it today ;)

  • If you think benchmarking with a NME number from the literature is tedious, we can drop it, and just rely on the fact we're testing the euclidean distances using different methods.

I think it's very interesting. I am just wondering how to compare with the NME numbers from the literature, if our calculation is based on one image.
Their NME should be over the given test dataset, if I didn't misunderstand. Should we predict the full dataset with model in this case?

@rabah-khalek
Copy link
Contributor

rabah-khalek commented Dec 12, 2023

Their NME should be over the given test dataset, if I didn't misunderstand. Should we predict the full dataset with model in this case?

Yes I was thinking of using the whole 300W dataset

@Inokinoki Inokinoki marked this pull request as ready for review December 18, 2023 18:10
@Inokinoki
Copy link
Member Author

It seems that face alignment returns different results in Python 3.10 (passed) and Python 3.11 (not passed):

=========================== short test summary info ============================
FAILED tests/test_benchmark_nmes.py::test_face_alignment_model - assert False
 +  where False = <function isclose at 0x7f6b13d32270>(0.06233510979950631, 0.06287962278002229)
 +    where <function isclose at 0x7f6b13d32270> = np.isclose
=================== 1 failed, 12 passed in 112.01s (0:01:52) ===================

@rabah-khalek rabah-khalek merged commit 005239a into main Dec 22, 2023
3 checks passed
@rabah-khalek rabah-khalek deleted the feature/gsk-2247-benchmark-the-nme-implementation branch December 22, 2023 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants