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

FastMtCnn Use of Gpu for Face Detection #1145

Merged
merged 5 commits into from
Apr 5, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion deepface/detectors/FastMtCnn.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,16 @@ def build_model(self) -> Any:
# this is not a must dependency. do not import it in the global level.
try:
from facenet_pytorch import MTCNN as fast_mtcnn
import torch
except ModuleNotFoundError as e:
raise ImportError(
"FastMtcnn is an optional detector, ensure the library is installed."
"Please install using 'pip install facenet-pytorch' "
) from e

face_detector = fast_mtcnn(device="cpu")
device = torch.device('cuda:0' if torch.cuda.is_available() else 'cpu')
face_detector = fast_mtcnn(device=device)
Copy link
Owner

Choose a reason for hiding this comment

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

to import fast_mtcnn, we enforce users to install facenet_pytorch instead of torch. not sure installing facenet_pytorch will install torch, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review and feedback on the pull request. I appreciate your concern regarding the addition of new dependencies and the potential impact on the package size and user experience.

Regarding the dependency on facenet_pytorch, it's important to note that while facenet_pytorch indeed does not directly require torch, it does require torchvision. The torchvision package, in turn, lists torch as one of its dependencies. Consequently, installing torchvision (as a requirement for facenet_pytorch) will ensure that torch is also installed if it is not already present in the environment.

This chain of dependencies means that users will end up with torch installed as part of the process, indirectly satisfying the requirement for torch without explicitly listing it as a direct dependency for our project.

I hope this clarifies the dependency chain and addresses your concern. I am open to further discussion and suggestions on how we can improve this implementation.

Best regards,

Copy link
Owner

@serengil serengil Mar 27, 2024

Choose a reason for hiding this comment

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

okay I can confirm that installing facenet-pytorch will install torchvision, and installing torchvision will install torch

(base) sefik@L-GCCDXDDT:~$ pip show facenet-pytorch
Name: facenet-pytorch
Version: 2.5.3
Summary: Pretrained Pytorch face detection and recognition models
Home-page: https://github.com/timesler/facenet-pytorch
Author: Tim Esler
Author-email: tim.esler@gmail.com
License:
Location: /home/sefik/miniconda3/lib/python3.9/site-packages
Requires: numpy, pillow, requests, torchvision
Required-by:


(base) sefik@L-GCCDXDDT:~$ pip show torchvision
Name: torchvision
Version: 0.15.2
Summary: image and video datasets and models for torch deep learning
Home-page: https://github.com/pytorch/vision
Author: PyTorch Core Team
Author-email: soumith@pytorch.org
License: BSD
Location: /home/sefik/miniconda3/lib/python3.9/site-packages
Requires: numpy, pillow, requests, torch
Required-by: facenet-pytorch, timm, ultralytics

But still putting import torch to the line 5 will throw exception for users who do not have facenet-pytorch. Because when i run pip install deepface, it will not install torch (it is not a must dependency) and when i import deepface as from deepface import DeepFace, then torch will be attempted to be imported but it will throw exception because I do not have that dependency. So, this change breaks deepface code as is.

As you can see facenet_pytorch is imported in line 64 instead of global level: https://github.com/serengil/deepface/blob/master/deepface/detectors/FastMtCnn.py#L64

You may consider to import torch in that try except block. Otherwise, I cannot merge this PR because it will break the deepface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You may consider to import torch in that try except block. Otherwise, I cannot merge this PR because it will break the deepface.

Sure, I'll do


return face_detector


Expand Down