-
Notifications
You must be signed in to change notification settings - Fork 45
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
Refactor mdr_find_people #152
Comments
After a quick discussion with @henrikschnor today, I think we can consolidate the different person recognition functionalities into a people recognition action server.
# assuming all the recognition models are Keras
recognition_models:
gender: /path/to/gender/model.h5
emotion: /path/to/emotion/model.h5
...
identity_model: /path/to/identity/model.h5
age_model: /path/to/age/model.h5 # doesn't exist yet
detection_models:
full_body_detection:
module: mas_image_detection.ssd_tensorflow
class: SSDTfModelsImageDetector
config_package: mas_image_detection
kwargs_file: configs/ssd_tensorflow_coco_kwargs.yml
class_file: configs/tf_models_coco_classes.yml
person_class: 1
key_point_detection:
# to be implemented still
model_path: /path/to/some/model
@argenos @alex-mitrevski @PatrickNa feel free to share thoughts on the topic |
At a quick glance, this seems like a solid improvement, particularly because the gender and emotion recognition actions were not really actions in the first place. I'd thus say go for it. P.S. You might want to think about actual person recognition as well, namely matching the detected people with known names. I see the |
Good point on the name model. Edited! @robertocaiwu has also expressed interest in working on this issue. |
A complete rewrite using things from |
This sounds good! Can we find a way to split this issue so both @robertocaiwu and @henrikschnor can work in parallel? |
After going through the rules again, there are three functionalities related to this: 1) the perception related part of detecting the person, 2) perception related part of recognition of different stuff and 3) the speech part of providing the description. @minhnh with this being mostly refactoring, can you take the lead? I think we need an initial version of a branch with the right structure, maybe then we can split the rest of the task between @robertocaiwu and @henrikschnor (depending on what comes out of the scenario tests) |
sure I'll work on this later this week |
* remove name & id fields in Face.msg in favor of identity field in Person.msg * remove safe_pose field in Person.msg, so safety logic will be isolated from data model * add comments about legacy field in Person.msg, but not removing for compatibility * remove unused message FaceList.msg * is motivated by discussion in b-it-bots/mas_domestic_robotics#152
* remove name & id fields in Face.msg in favor of identity field in Person.msg * remove safe_pose field in Person.msg, so safety logic will be isolated from data model * add comments about legacy field in Person.msg, but not removing for compatibility * remove unused message FaceList.msg * is motivated by discussion in b-it-bots/mas_domestic_robotics#152
As discussed in #145, we need further discussion in how to handle
mdr_find_people
.The text was updated successfully, but these errors were encountered: