Conversation
vfdev-5
left a comment
There was a problem hiding this comment.
@louis-she thanks a lot for the PR !
This is a great start for finally adding detection example to ignite !
I left some comments on how to improve some parts of it...
| @visualizer.on(Events.ITERATION_COMPLETED) | ||
| def add_vis_images(engine): | ||
| engine.state.model_outputs.append(engine.state.output) | ||
|
|
||
| @visualizer.on(Events.ITERATION_COMPLETED) | ||
| def submit_vis_images(engine): | ||
| aim_images = [] | ||
| for outputs in engine.state.model_outputs: | ||
| for image, target, pred in zip(outputs["x"], outputs["y"], outputs["y_pred"]): | ||
| image = (image * 255).byte() | ||
| pred_labels = [Dataset.class2name[l.item()] for l in pred["labels"]] | ||
| pred_boxes = pred["boxes"].long() | ||
| image = draw_bounding_boxes(image, pred_boxes, pred_labels, colors="red") | ||
|
|
||
| target_labels = [Dataset.class2name[l.item()] for l in target["labels"]] | ||
| target_boxes = target["boxes"].long() | ||
| image = draw_bounding_boxes(image, target_boxes, target_labels, colors="green") | ||
|
|
||
| aim_images.append(aim.Image(image.numpy().transpose((1, 2, 0)))) | ||
| aim_logger.experiment.track(aim_images, name="vis", step=trainer.state.epoch) |
There was a problem hiding this comment.
I'm not quite sure to understand why we need to accumulate and submit on iterations...
Can't we just submit on iterations without accumulating ?
There was a problem hiding this comment.
Can submit on iterations only if we set step as trainer.state.iteration, or aim will override the previous images.
Gather visualization images in epoch can give a good view when using aim I think, but indeed it will eat more memory.
| @@ -0,0 +1,252 @@ | |||
| """ | |||
There was a problem hiding this comment.
Just a comment for ourselves that we have to advance on implementing mAP from our side
| def synchronize_between_processes(self): | ||
| for iou_type in self.iou_types: | ||
| self.eval_imgs[iou_type] = np.concatenate(self.eval_imgs[iou_type], 2) | ||
| create_common_coco_eval(self.coco_eval[iou_type], self.img_ids, self.eval_imgs[iou_type]) |
There was a problem hiding this comment.
This method looks a bit weird as according to its name it should sync between procs...
| labels = targets["labels"].tolist() | ||
| areas = targets["area"].tolist() | ||
| iscrowd = targets["iscrowd"].tolist() | ||
| if "masks" in targets: |
There was a problem hiding this comment.
Here, the implementation could be simplified since no masks neither key points are used.
| parser.add_argument("--image-size", type=int, default=512, help="image size to train") | ||
| parser.add_argument("--experiment-name", type=str, default="test", help="name of one experiment") | ||
| args = parser.parse_args() | ||
| run( |
There was a problem hiding this comment.
idist could be used for ddp
There was a problem hiding this comment.
I don't have 2 GPUs to test distribution now. I'll try to write the distribution code and, maybe test on AWS?
| name2class = {v: k + 1 for k, v in enumerate(classes)} | ||
| class2name = {k + 1: v for k, v in enumerate(classes)} | ||
|
|
||
| def __getitem__(self, index: int) -> Tuple[Any, Any]: |
There was a problem hiding this comment.
It seems that what is done here could be implemented as a transformation. It should be moved in another file IMO.
There was a problem hiding this comment.
Do you mean the name and class number conversion? If yes I think it's ok to place here since there are nothing to do with the images.
|
Also, the tests are failing because of code-formatting error, try running |
examples/contrib/detection/main.py
Outdated
| with torch.cuda.amp.autocast(enabled=True): | ||
| loss_dict = model(images, targets) | ||
| loss = sum(loss for loss in loss_dict.values()) |
There was a problem hiding this comment.
Still cuda.amp.autocast, can it work on CPU ?
examples/contrib/detection/main.py
Outdated
| engine.state.model_outputs.append(engine.state.output) | ||
|
|
||
| @visualizer.on(Events.ITERATION_COMPLETED) | ||
| def submit_vis_images(engine): |
There was a problem hiding this comment.
Does this ok in distributed ? I would have added a @one_rank_only() decorator.
There was a problem hiding this comment.
I havn't tested it on multi gpu core yet, there is a @one_rank_only() protection on the trainer's epoch_end so the visualizer engine will not be called on other ranks.
|
@vfdev-5 @sdesrozis |
|
One of the ci is failed cause |
|
I see that there is a PR of Can I open a new PR to implemente mAP? |
examples/contrib/detection/README.md
Outdated
| ## Usage: | ||
|
|
||
| ```bash | ||
| python main.py exp_name |
There was a problem hiding this comment.
It is a bit unclear what is exp_name here. We may do something like
python main.py <exp_name>
# for example
# python main.py frcnn
examples/contrib/detection/main.py
Outdated
| local_rank: int, | ||
| device: str, | ||
| experiment_name: str, | ||
| gpus: Optional[Union[int, List[int], str]] = None, |
There was a problem hiding this comment.
Looks like gpus is unused ?
An image detection example using PyTorch Faster RCNN with Aim. Some screenshots: