search

yosungho / LineTR

Line as a Visual Sentence: Context-aware Line Descriptor for Visual Localization (Line Transformer)
Other
233 stars 35 forks source link

Potential bug in the KeylineEncoder #4

Closed rpautrat closed 1 year ago

rpautrat commented 2 years ago

Hi @yosungho,

Thanks for this very nice work and for sharing the code, it is very insightful!

When studying the code, I noticed something weird that might be a bug. It is located in the KeylineEncoder at line: https://github.com/yosungho/LineTR/blob/afd89effcc081fe75105ea9c3598fdc5e91b6179/models/line_transformer.py#L121 It seems that the line descriptor is not updated at each iteration, since the output of the desc_layer has a different name than the input (desc vs enc_output). This does not trigger any error from the compiler, but the effect is that only one iteration of desc_layers is effectively used... The previous iterations are just overwritten each time.

Let me know if I got a wrong understanding of this line. Otherwise I think that the correction would be the following:

for desc_layer in self.desc_layers:
   desc, enc_slf_attn = desc_layer(desc, slf_attn_mask=mask)
   enc_slf_attn_list += [enc_slf_attn] if return_attns else []
enc_output = desc
yosungho commented 2 years ago

Yes, I think you are right. Thanks for reporting it. I will investigate this issue more next week.

maengrui commented 2 years ago

an nyeong ha se yo,jeon neun maeng rui im ni da,when im learning the NLP in my projects, i found your paper is a new way in visual detect ,im trying to learn your code ,thank you for your sharing!