Batching on .extract_faces to improve performance and utilize GPU in full
#1435
Conversation
|
Do you have a branch in your fork that currently combines all the optimizations you've submitted. I'd like to start using them while the approval process is going whats been the total speedup you've been able to see |
|
I do. You can check it combines these two PRs with some other small modifications:
Not all of the detectors currently (both in this PR and in the fork) implement batching. In particular, YOLO does. I've found it to be optimal in terms of performance and inference speed. The only problem is installing both torch and tensorflow with GPU, but I've managed to somehow do that. All in all, with the combination of @serengil FYI I would be happy to contribute the aforementioned modifications if we have progress on the PRs. |
|
I will review this PR this week i hope |
|
Seems this breaks the unit tests. Must be sorted. |
|
should be good now |
|
Nope, still failing. |
|
You implemented OpenCv, Ssd, Yolo, MtCnn and RetinaFace to accept list inputs What if I send list to YuNet, MediaPipe, FastMtCnn, Dlib or CenterFace? I assume an exception will be thrown, but users should see a meaningful message. |
|
To summarize what's changed:
Unfortunately, this didn't fix the batch extraction case on
|
|
If detector is opencv or mtcnn, and input is batch, then we should raise an error. We don't need ENABLE_OPENCV_BATCH_DETECTION or ENABLE_MTCNN_BATCH_DETECTION environment variables. TBH, i don't want to make this optional to users. They are raising issues when they have something is wrong. |
|
Please add unit test cases for opencv and mtcnn. When batch input is sent, then exception should be thrown. |
|
I don't like this approach: all detectors should have |
…s based on process_single_image method" This reverts commit 8c7c2cb.
|
Changes:
so now they just use a for loop to process all the images, if a batch is passed. A user gets a warning that there is no actual batching happening. Do you still think it is a good idea to raise an error? |
| @@ -1,5 +1,6 @@ | |||
| # built-in dependencies | |||
| from typing import List | |||
| import logging | |||
There was a problem hiding this comment.
we are not using logging module directly anywhere. use this instead:
from deepface.commons.logger import Logger
logger = Logger()
| @@ -8,6 +9,8 @@ | |||
| # project dependencies | |||
| from deepface.models.Detector import Detector, FacialAreaRegion | |||
|
|
|||
| logger = logging.getLogger(__name__) | |||
| @@ -1,6 +1,7 @@ | |||
| # built-in dependencies | |||
| import os | |||
| from typing import Any, List | |||
| from typing import Any, List, Union | |||
| import logging | |||
| resp.append(facial_area) | ||
|
|
||
| return resp | ||
| is_batched_input = isinstance(img, list) |
There was a problem hiding this comment.
you are writing same code in every detector
is_batched_input = isinstance(img, list)
if not is_batched_input:
imgs = [img]
else:
imgs = imgIMO, this should be done in the parent where those detectors are being called. In that way, we will not have repeated code.
|
|
||
| batch_results.append(resp) | ||
|
|
||
| if not is_batched_input: |
There was a problem hiding this comment.
same here, we can do this in parent and avoid repeated code.
if not is_batched_input:
return batch_results[0]
return batch_results
|
|
||
| resp.append(facial_area) | ||
| img_rgb = [img[:, :, ::-1] for img in img] | ||
| if self.supports_batch_detection: |
There was a problem hiding this comment.
self.supports_batch_detection is returning false always. so, get rid of _supports_batch_detection.
have just this
detections = [self.model.detect_faces(single_img) for single_img in img_rgb]
| @@ -29,55 +32,72 @@ def build_model(self): | |||
| detector["eye_detector"] = self.__build_cascade("haarcascade_eye") | |||
| return detector | |||
|
|
|||
| def detect_faces(self, img: np.ndarray) -> List[FacialAreaRegion]: | |||
| def _supports_batch_detection(self) -> bool: | |||
There was a problem hiding this comment.
no need _supports_batch_detection, it is returning false always
| detected_face = None | ||
| if isinstance(img, np.ndarray): | ||
| imgs = [img] | ||
| elif self.supports_batch_detection: |
There was a problem hiding this comment.
this condition is always false, get rid of it
| @@ -79,6 +83,169 @@ def test_different_detectors(): | |||
| logger.info(f"✅ extract_faces for {detector} backend test is done") | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("detector_backend", [ | |||
| # "opencv", | |||
There was a problem hiding this comment.
activate opencv please, it is default detector. important to have tests.
| expected_num_faces = [1, 1, 1, 2] | ||
|
|
||
| # load images as numpy arrays | ||
| imgs_batch = np.stack(imgs, axis=0) |
There was a problem hiding this comment.
please add this control
assert imgs_batch.ndim == 4 and imgs_batch.shape[0] == 4
| img_path=[img_path], | ||
| align=True, | ||
| ) | ||
| assert len(imgs_objs_batch) == 1 and isinstance(imgs_objs_batch[0], list) |
There was a problem hiding this comment.
please check this
assert len(imgs_objs_batch[0]) == 2
for loop is okay, don't raise error |
|
As I understand, you built a for loop in each detector. Is that really a batch? What if we build a for loop in the parent of detectors, and don't change detectors? |
|
I mean batch is really running for yolo We can create a method extract faces from batch in parent class of detectors, and only customize this for yolo. In parent class, we basically build a for loop and call detector In that way, detectors will not change |
Tickets
#1433
#1101
#1434
What has been done
With this PR,
.extract_facesis able to accept a list of imagesHow to test
Benchmarking on detecting 50 faces:
For yolov11n, batch size 20 is 59.27% faster than batch size 1.
For yolov11s, batch size 20 is 29.00% faster than batch size 1.
For yolov11m, batch size 20 is 31.73% faster than batch size 1.
For yolov8, batch size 20 is 12.68% faster than batch size 1.