facebookresearch / KILT

Library for Knowledge Intensive Language Tasks
MIT License
910 stars 91 forks source link

Integration with DPR Repository #22

Closed anthonywchen closed 3 years ago

anthonywchen commented 3 years ago

Hello! Thank you for the great repository/paper. I noticed some issues with how some of the DPR_connector code interacts with the DPR repository.

  1. This line leads to an error because DenseRetriever class in the DPR repo doesn't have that function. I believe that this line should be changed to: self.retriever.index.index_data(input_paths)

  2. This is really minor: In the Retriever README there is this instruction:

change line 185 of src/dpr/dense_retriever.py to

try:
    db_id, doc_vector = doc
except:
    title, db_id, doc_vector = doc

except that this isn't line 185 anymore.

Thanks again for the great work!

Best, Anthony

fabiopetroni commented 3 years ago

Hey @anthonywchen, if you can include also this fix in your pull request would be great! :)

anthonywchen commented 3 years ago

Sure :)

yuwfan commented 3 years ago

Thanks @anthonywchen and @anthonywchen. I opened a PR for this issue: https://github.com/facebookresearch/KILT/pull/29.

anthonywchen commented 3 years ago

I noticed that making this change isn't necessary to get DPR to work:

try:
    db_id, doc_vector = doc
except:
    title, db_id, doc_vector = doc

I can remove this from the retriever README when making the pull request for the other issue

fabiopetroni commented 3 years ago

THANKS A LOT ! :)