Haystack: Different document embedding values between DPR repository and Haystack DPR implementation

[antoniolanza1996]

Describe the bug
I have tried to embed the same document using both DPR repository (as described here) and DensePassageRetriever.

For both the strategies, I have used the same pre-trained model published by DPR authors here.

However, if I compare embedding representations, they are different.
Why does it happen? shouldn't the same results come out?

To Reproduce
I have updated a notebook here to better explain the bug. Hopefully, it could help you to find the problem.

System:

• GPU with Google Colab
• Haystack version: latest
• DocumentStore: ElasticsearchDocumentStore
• Retriever: DensePassageRetriever

[Timoeller]

Hey thanks for the detailed report.
Currently haystacks document_store.update_embeddings(retriever) does not support embedding the title. See the snippet how the documents are embedded:

docs = self.get_all_documents(index)
passages = [d.text for d in docs] # only "text" and not title ("name") is being indexed
embeddings = retriever.embed_passages(passages) 

So I think the difference comes from the missing title. You could try embedding questions (they dont have titles) and compare both vectors. I would be very surprised though if they differ.

We are currently refactoring DPR completely and also getting DPR training into haystack in #273
There we will add the feature of embedding the title as well, because it makes a lot of sense of course.

If you agree with what I just wrote, could you change the title of the issue to reflect the missing title in document embeddings? If you need a solution more quickly I could also help you creating a PR with the functionality.

[antoniolanza1996]

Hi @Timoeller, thanks for your detailed reply.

I have tried to remove title in DPR repository setting insert_title = False here. In this manner, I think, both DPR and Haystack should consider only the text document.

The values are still not the same, however it would seem that they are closer to Haystack DPR implementation.

1. DPR generate_dense_embeddings.pywith insert_title=True -> [ 0.01334194 0.5821738 -0.31309715 -0.69916165 -0.55831945]
2. DPR generate_dense_embeddings.py withinsert_title=False -> [ 0.04298835 0.6609285 -0.24494183 -0.7152531 -0.53992623]
3. Haystack DPR implementation -> [0.05509451404213905, 0.6544742584228516, -0.1710992306470871, -0.7139490246772766, -0.4473816156387329]

I think we can wait for your refactoring and after we can re-run these tests. What do you think?

[Timoeller]

Ok getting close but not quite. Interesting.

Do you have an explanation for this small difference?
What about max_seq_len, are they the same for both methods? I slightly doubt that something inside the architecture is different, since we based our code in dpr_utils.py on the fb code...

[antoniolanza1996]

Hi @Timoeller ,
I have deeply investigated on it. In particular, I have noted that the problem comes up because in the original Wikipedia split by DPR there are some " in the text replaced with "". These are encoded by csv.reader (here) with only a single ", but Elasticsearch encodes them as two different ". Hence, when a document has fed into a tokenizer, clearly, it returns lower tokens for the DPR original implementation. At this point, I think that's what leads to the slight difference in value seen before.

I have also tried a different doc without " and the final embedding result is exactly the same (clearly considering only the text during embedding and excluding the title).

In conclusion, there are no problems in Haystack DPR implementation.

[Timoeller]

Wow crazy deep dive and good to know about these subtle differences. Thanks for sharing with the community.

What are your plans for using haystack if I might ask?

[antoniolanza1996]

Hi @Timoeller,
It was really strange and I didn't explain why. Thanks also to you for giving me the hint of not considering the title during embedding.

I am going to close this issue, even though, as you said before, it could be really good if you integrate the title during embedding. It's only 1 line of code in the actual version of dpr_utils.py (1). Hopefully, it doesn't become too difficult to integrate it into your refactoring in #273.

About your question, I'm using Haystack for my Computer Engineering master thesis.

(1) During coding, I have noted that we should modify dense.py to stay consistent with Haystack logic (e.g. __generate_batch_predictions_ used both for query and passages).

[Timoeller]

Nice. Testing large scale QA systems in your thesis? If you need some sparring or additional info to haystack we could setup a short call.

And about the title integration. We would be more than happy about contributions. The current DPR refactoring would not touch the document store much, so adding it there and in dpr utils would be helpful and used in the future. What do you think? :)

[antoniolanza1996]

Hi @Timoeller ,

Nice. Testing large scale QA systems in your thesis? If you need some sparring or additional info to haystack we could setup a short call.

Thank you very much, I appreciate it. If needed, I'll get in touch with you in the future.

And about the title integration. We would be more than happy about contributions. The current DPR refactoring would not touch the document store much, so adding it there and in dpr utils would be helpful and used in the future. What do you think? :)

Ok, I have quickly integrated it. I have added major comments directly in the PR #298. Hoping it will be ok.