RSDK-6381 - Add optional label renaming to detection transform camera #3538
Conversation
viambot
added
the
safe to test
There was a problem hiding this comment.
I have a few changes I'd like you to do, as well as some extra things:
- Can you also create the post-processor for classifiers. It's essentially the same code, and it would extremely frustrating for someone switching between using a classifier and a detector if sometimes the relabeling worked, and sometimes not.
- Can you add tests to both the classifier and detector re-label function
- Can you confirm that it works on detectors by trying it out manually
| DetectorName string `json:"detector_name"` | ||
| ConfidenceThreshold float64 `json:"confidence_threshold"` | ||
| ValidLabels []string `json:"valid_labels"` | ||
| LabelRenamer map[string]string `json:"label_renamer,omitempty"` |
There was a problem hiding this comment.
by making the field an agent noun, it makes it seem to me like the config requires a function- I would prefer a name like "rename_labels"
| // NewLabelRenamer renames the labels in the input map from the key to the value. | ||
| func NewLabelRenamer(labels map[string]string) Postprocessor { | ||
| return func(in []Detection) []Detection { | ||
| if len(labels) < 1 { |
There was a problem hiding this comment.
I learned today that you can call len() on a nil map and it will not panic - thanks for this
| for oldL, newL := range labels { | ||
| for i, d := range in { | ||
| if strings.HasPrefix(strings.ToLower(d.Label()), strings.ToLower(oldL)) { | ||
| in[i] = NewDetection(*d.BoundingBox(), d.Score(), newL) |
There was a problem hiding this comment.
This also modifies the input slice that was fed into the function, and I don't think you want to do that. You should make a new output slice and put the NewDetections in that, and then return "out".
| for oldL, newL := range labels { | ||
| for i, d := range in { |
There was a problem hiding this comment.
I dislike the fact that you have to iterate over the entire map every time, but I understand due to the prefix matching that you have to. I suggest an alternative though, because I think ALWAYS mapping to a prefix can cause problems (e.g. if my detector knows about "car" or "carrot" or "carnations" it will catch all of them even if I just wanted to replace "car")
The alternative:
- if there is a
*at the end of the key in the label map, then you do a prefix match - fi there is no
*at the end of the key in the label map, you do an exact match
When first defining NewLabelRenamer, you split the map into two maps, the exact matcher map, and the prefix matcher map. Then you don't have to iterate over all of the labels all the time.
No description provided.