IDEA-Research / DINO

[ICLR 2023] Official implementation of the paper "DINO: DETR with Improved DeNoising Anchor Boxes for End-to-End Object Detection"
Apache License 2.0
2.1k stars 230 forks source link

Regarding Look Ahead Twice Implementation #185

Open RaviSriTejaKuriseti opened 1 year ago

RaviSriTejaKuriseti commented 1 year ago

In the reply for the issue #77 , you have mentioned that the line https://github.com/IDEACVR/DINO/blob/8758cf02146f306dc36babab4fff1f09c114c682/models/dino/deformable_transformer.py#L773 implements the look ahead twice updates for box.

When we look at the below diagram in the original paper(Fig-6 in the original paper), we see that there is a gradient detach between the boxes $b'_{i-1}, b_{i-1}$ for the case of look forward twice. image.

In the code at the link https://github.com/IDEACVR/DINO/blob/8758cf02146f306dc36babab4fff1f09c114c682/models/dino/deformable_transformer.py#L77 It was mentioned that when we toggle the rmdetach value it activates the look forward twice. But in the true part of the if condition we observe that the code creates a direct link between $b'_{i-1}, b{i-1}$ which shows that there is no detach for the look forward twice case which is contrasting to what you have mentioned in the paper.

And this ideally means that that the gradient is passing through all the previous values.

image In that case, the update rule in the above equation (2) make no sense because $b'_{i}, {b_{i}}^\text{(pred)}$ are the same.

Maybe we are missing out on something. Could you please clarify this?

AshStuff commented 1 year ago

The issue #77 says that the look ahead twice is controlled by rm_detach. However, It is set to None. So i think detach happens at that place.

ajayshastry08 commented 1 year ago

@AshStuff The case that you are mentioning is look forward once. If you observe Figure 6 (a) in the paper it shows that look forward once also has a gradient detach.

AshStuff commented 1 year ago

I am confused then. In the code, rm_detach is set to None (hard coded). So https://github.com/IDEA-Research/DINO/blob/f5bd5474b1dafc15eab72016eed7f55b51a8ea69/models/dino/deformable_transformer.py#L746 line doesn't happen at all.

ajayshastry08 commented 1 year ago

If we set the rm_detach to ['dec'] rather than None, then it goes to the positive side of the if condition

HaoZhang534 commented 1 year ago

@AshStuff @RaviSriTejaKuriseti @ajayshastry08 Sorry, it should be this line https://github.com/IDEA-Research/DINO/blob/8758cf02146f306dc36babab4fff1f09c114c682/models/dino/deformable_transformer.py#L777 that implements look forward twice.