dki-lab/GrailQA

Suspicious bug in logical form generation

xiye17 opened this issue · 2 comments

Hi,
bert_seq2seq_reader.py: line 203 seems to be suspicious. Is it a bug or a feature?
Could it be lfs_2 = generate_all_logical_forms_2(entities, offline=self._offline)?
It is somewhat unnatural to iterate through entities but always call generate_all_logical_forms_2 using entities[0].

Thanks for pointing it out! I think it was a design choice by me mainly because that for BERT-based model it is more time-consuming to rank all candidate queries by iterating all the entities. I don't remember clearly how the numbers of iterating all the entities are compared with only using the first entity, but it is straightforward to try that because the ranking-based model is trained in the same way as the transduction-based model (which means candidate generation is not relevant to the training process), so we can directly do ranking on a different set of candidates during inference.

Thanks! I see the point. That makes sense. But line 203 will introduce duplicate logical forms anyway. Then, should it be moved out of the loop?

Btw, if we only generate the logical forms for the first candidate, does it make sense to also only call generate_all_logical_forms_alpha on entities[0] rather than iterating through all entities. (I mean delete line 201 and use entities[0] for line 202'.