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

alignment_procedure crashes when both eyes are in the same position #94

Closed
Dobiasd opened this issue Mar 20, 2024 · 9 comments
Closed
Labels
bug Something isn't working

Comments

@Dobiasd
Copy link
Contributor

Dobiasd commented Mar 20, 2024

When looking at a face from the side, the landmark detector may give the same position for both eyes.

In that case, retinaface.commons.postprocess.alignment_procedure crashes with UnboundLocalError: cannot access local variable 'angle' where it is not associated with a value.

Some IDEs also highlight this problem:

IPjqZfD

Here is a minimal example to reproduce the problem:

import numpy as np
from retinaface.commons import postprocess

postprocess.alignment_procedure(np.zeros([123, 123, 3], dtype=np.uint8), (10, 10), (10, 10), (33, 33))
@Dobiasd
Copy link
Contributor Author

Dobiasd commented Mar 20, 2024

Of course, it would be nice if pylint would catch such problems already in the CI.

I've opened a StackOverflow question asking if enabling such a check is possible.

@serengil serengil added the bug Something isn't working label Mar 20, 2024
@serengil
Copy link
Owner

Good spot! Do you mind to share your input image? Would be easier to fix if i can reproduce.

@Dobiasd
Copy link
Contributor Author

Dobiasd commented Mar 20, 2024

It happened while processing a video stream. I no longer have the problematic frame, but I'll try to reproduce one by moving in front of my webcam. 🕺

@Dobiasd
Copy link
Contributor Author

Dobiasd commented Mar 20, 2024

I've tried, but so far failed to align my head in a pose to fulfill the required condition.

Hope you can maybe fix it even without such an example image.

@serengil
Copy link
Owner

No problem, we can still fix it.

@serengil
Copy link
Owner

@Dobiasd if we set an initial 0 value to angle in alignment produce function, then this will be sorted.

Do you mind to create a PR for this? I would like to see your name as a contributor because you found this issue.

https://github.com/serengil/retinaface/blob/master/retinaface/commons/postprocess.py#L33

serengil added a commit that referenced this issue Mar 20, 2024
Fix UnboundLocalError in alignment_procedure, closes #94
@Dobiasd
Copy link
Contributor Author

Dobiasd commented Mar 21, 2024

FYI:

While pylint is not able to catch such bugs in the linter step, mypy (with --enable-error-code possibly-undefined) is. 🎉

So in case you're interested in preventing such issues automatically in the future, using mypy could be an option. 🙂

@serengil
Copy link
Owner

Oh no, very satisfied with pylint, do not want to switch anything else :)

@Dobiasd
Copy link
Contributor Author

Dobiasd commented Mar 21, 2024

Ah, no, I did not mean to replace pylint with mypy, but adding mypy to the CI. mypy is a type checker and solves different problems than pylint (a linter) does. Not only our team at work, but also me in my private projects (example) use both in conjunction, and I like this combination a lot. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants