Open jklontz opened 10 years ago
Looks good. One thought:
Given the confusion that many users have, we should consider having no default setting. That is, the user must specify which case they want to use, otherwise a meaningful error message would be displayed stating that one of these options must be chosen. I think without this behavior there will continue to be many issues posted to the mailing list.
One related point is that expand does two kind of contradictory things. It expands multiple detections per template into multiple templates, and also drops fte etc. templates. These are largely two separate goals and i think it would be clearer if there were 2 separate transforms
- N = 1, enroll exactly one template per image no matter what. This is the current default behavior (corresponding to enrollAll = false) and is useful for biometric evaluations where we need a fixed gallery / simmat size.
Right now this always "enrolls something" even if it is fails all face detection, defaults to the whole image, better name that captures this?
This should differ from the case where I only want to enroll a single "good" face, without going down a quality detect rabbit hole, what about an enroll best or enroll largest?
Enroll nothing, anything, best face, all faces
One more thing to consider, we might be better served with booleans like,
LARGEST|SINGLEFACE
- N >= 0, enroll however many templates are detected in the image. This is the current alternative behavior (corresponding to enrollAll = true) and is useful for most practical applications where we want to enroll anything that is detected.
However, it's also desirable to support:
- N >= 1, enroll however many templates are detected in the image, but always enroll at least one template no matter what. This mode is desirable in our client/server case, where we'd always like to send a response, but could potentially want to send multiple responses.
- N = [0,1], enroll one template if something is detected, otherwise enroll nothing. Useful for controlled capture enrollment into a database.
Seeking to maintain , I propose we change the enrollAll boolean to an enum:
enum EnrollmentState { EnrollOne = 0, // N = 1, remains the default behavior EnrollNone = 1, // N = [0, 1], new option EnrollMany = 2, // N >= 1, new option EnrollAny = EnrollNone + EnrollMany // N >= 0, currently enrollAll = true };
br would then be updated to support the following 4 flags which change this global enrollment state:
- -enrollOne
- -enrollNone
- -enrollMany
- -enrollAny
Finally, detectors would check for the presence of EnrollNone indicating that they don't necessarily need to return a detection, and EnrollMany indicating that they can return more than one detection if they like.
Thoughts? Does this actually address the perceived issues with enrollAll?
— Reply to this email directly or view it on GitHub https://github.com/biometrics/openbr/issues/211.
I'd prefer to avoid any modality-specific names/flags.
Another point is that things like which face the detector should use (e.g. select largest, select according to some quality metric, or maybe even single vs. multiple detections) may be better represented as a property of the face detector rather than a global flag.
On the other hand, one issue with that approach is that due to limitations of the algorithm grammar, an abbreviation like "FaceDetection" cannot have any free parameters, so in e.g. any pre-trained algorithm, switching detection modes via a property of the detector is in fact impossible.
I agree with the points Charles laid out. The second point relates to https://github.com/biometrics/openbr/issues/194
On Wed, Jun 25, 2014 at 1:14 PM, caotto notifications@github.com wrote:
Another point is that things like which face the detector should use (e.g. select largest, select according to some quality metric, or maybe even single vs. multiple detections) may be better represented as a property of the face detector rather than a global flag.
On the other hand, one issue with that approach is that due to limitations of the algorithm grammar, an abbreviation like "FaceDetection" cannot have any free parameters, so in e.g. any pre-trained algorithm, switching detection modes via a property of the detector is in fact impossible.
— Reply to this email directly or view it on GitHub https://github.com/biometrics/openbr/issues/211#issuecomment-47131350.
It is now possible to supply additional arguments to pre-trained algorithms, or abbreviations. There are some limitations, e.g. only the first matching parameter will be set (if an algorithm had two instances of the same transform, it would only be possible to set parameters of the first one).
Even within current limitations, I think it's worth discussing whether or not a global flag is the best way to decide detector behavior at run time. Personally I would prefer to use parameters on detector transforms, rather than relying on a global flag which has to enumerate every possible scenario.
This is a good point. It would indeed seem more in the spirit of openbr to implement this with in the transforms. On Jul 11, 2014 4:24 PM, "caotto" notifications@github.com wrote:
It is now possible to supply additional arguments to pre-trained algorithms, or abbreviations. There are some limitations, e.g. only the first matching parameter will be set (if an algorithm had two instances of the same transform, it would only be possible to set parameters of the first one).
Even within current limitations, I think it's worth discussing whether or not a global flag is the best way to decide detector behavior at run time. Personally I would prefer to use parameters on detector transforms, rather than relying on a global flag which has to enumerate every possible scenario.
— Reply to this email directly or view it on GitHub https://github.com/biometrics/openbr/issues/211#issuecomment-48778617.
I'm on board with moving them from globals into transforms. But I think we still need to decide on a common set of parameter names and behaviors so that they can be implemented consistently across detectors. Perhaps another reason to build an abstract class for detectors?
This is a long standing blemish that needs refactoring. It's generally agreed that we have two orthogonal enrollment modes:
N = 1
, enroll exactly one template per image no matter what. This is the current default behavior (corresponding to enrollAll = false) and is useful for biometric evaluations where we need a fixed gallery / simmat size.N >= 0
, enroll however many templates are detected in the image. This is the current alternative behavior (corresponding to enrollAll = true) and is useful for most practical applications where we want to enroll anything that is detected.However, it's also desirable to support:
N >= 1
, enroll however many templates are detected in the image, but always enroll at least one template no matter what. This mode is desirable in our client/server case, where we'd always like to send a response, but could potentially want to send multiple responses.N = [0,1]
, enroll one template if something is detected, otherwise enroll nothing. Useful for controlled capture enrollment into a database.Seeking to maintain , I propose we change the
enrollAll
boolean to an enum:br
would then be updated to support the following 4 flags which change this global enrollment state:Finally, detectors would check for the presence of
EnrollNone
indicating that they don't necessarily need to return a detection, andEnrollMany
indicating that they can return more than one detection if they like.Thoughts? Does this actually address the perceived issues with
enrollAll
?