Skip to content
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

implement IOU Loss #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hamadichihaoui
Copy link

@hamadichihaoui hamadichihaoui commented Jul 11, 2020

Based on:
https://github.com/Zzh-tju/DIoU-SSD-pytorch/blob/86a370aa2cadea6ba7e5dffb2efc4bacc4c863ea/utils/box/box_utils.py#L47
Distance-IoU Loss: Faster and Better Learning for Bounding Box Regression
https://arxiv.org/pdf/1911.08287.pdf
Generalized Intersection over Union: A Metric and A Loss for Bounding Box Regression
https://giou.stanford.edu/GIoU.pdf
UnitBox: An Advanced Object Detection Network
https://arxiv.org/pdf/1608.01471.pdf

I made a quick test using the global wheat detection dataset, the AP@0.5..0.75 reaches 0.35 and stop to increase.
@rwightman

@hamadichihaoui hamadichihaoui changed the title implement iou_loss implement IOU Loss Jul 11, 2020
@rwightman
Copy link
Owner

@hamadichihaoui thanks for the contribution, I will evaluate the iou loss with my next training experiments before adding

One question, the reference impl has a GPL license so code can not be directly brought in here without changing my Apache 2.0 license (which I do not intend to do). Did you implement your PR from scratch without using any of the original code directly?

@hamadichihaoui
Copy link
Author

@rwightman Yes, I followed the other code but I implemented it from scratch.

@rwightman
Copy link
Owner

@hamadichihaoui great, thanks for confirming. I'll update here when I get some results training on COCO.

@hamadichihaoui
Copy link
Author

@rwightman Thanks, I will be waiting for your updates!

@liaopeiyuan
Copy link

Looking forward to the results! I implemented my own version of CIoU/DIoU loss but I couldn't obtain similar results on COCO when compared to huber loss.

@hamadichihaoui
Copy link
Author

@liaopeiyuan what AP you got on COCO?

@liaopeiyuan
Copy link

I don't think my stats are comparable because I changed a lot of the original code.

@liaopeiyuan
Copy link

Also, I haven't looked at your implementation, but AFAIK CIoU loss/DIoU loss only operates on xyxy/yxyx inputs, which is a lot different from EfficientDet's xy(log w)(log h)-relative format. How did you address this problem in your code? In my code, I chose to maintain box predictions as relative (to anchors), but decoded the raw output into xyxy format (with the exp function applied), then calculated the IoU losses for each outputs from the boxnet. but this seems less ideal. I wonder if @rwightman may have a more natural adaptation to this codebase.

@hamadichihaoui
Copy link
Author

I addressed it the same way, maybe there is a more elegant way to implement it.

@zlyin
Copy link

zlyin commented Jul 25, 2020

Hi Ross, any updates in this thread, @rwightman?

Hi @hamadichihaoui @liaopeiyuan, may I ask how you guys decode the gt_boxes & pred_boxes when calculating the loss?

  • Currently, both my gt_boxes & pred_boxes from the model are in yxyx relative to the anchors. I just use these relative values directly to calculate the loss. Though it converges well, but the model performances is pretty bad.
  • Did you use anchor sizes to recover the absolute xyxy format of boxes? If so, how did you retrieve the anchor sizes while calculating the loss?
  • As mentioned in your post above, what's the difference between raw outputs & box_predictions? Shouldn't the loss function consume gt_boxes & pred_boxes?

Thanks for your help!

@rwightman
Copy link
Owner

@zlyin nope, no major updates, started looking at the code, paper but haven't finished either.

@hamadichihaoui
Copy link
Author

hamadichihaoui commented Jul 25, 2020

@zlyin I used decode_box_outputs to recover the absolute xyxy format of boxes. The method takes 3 arguments, rel_codes= the output of the models in the relative format (raw outputs), anchors, output_xyxy= whether to get the absolute boxes in xyxy or yxyx.
The idea is just rel_codes and the anchors (which are both list) must match meaning that the rel_codes[0] should correspond to anchors[0] and so on.

@zlyin
Copy link

zlyin commented Jul 28, 2020

@hamadichihaoui, thank you for your help! I have implemented the new loss_function. Thanks!


if self.use_iou_loss:
# apply bounding box regression to anchors
for k in range(box_outputs_list.shape[0]):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems a bug, box_outputs_list is a list, has not shape attribute

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe I did a mistake, for k in range(len(box_outputs_list)):

if self.use_iou_loss:
# apply bounding box regression to anchors
for k in range(box_outputs_list.shape[0]):
pred_boxes = decode_box_outputs(box_outputs_list[k].T.float(), self.anchors.boxes.T, output_xyxy=True)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also a bug, ty, tx, th, tw = rel_codes.unbind(dim=1)
ValueError: too many values to unpack (expected 4)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check it, I think I was not working with the repo's last commit

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your reply, looking forward your new correct code

Copy link

@cswwp cswwp Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind share your variable giou loss with effdet version? @hamadichihaoui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants