Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added score tie-breaking to BM25PrfReranker #777

Merged
merged 3 commits into from
Aug 14, 2019

Conversation

emmileaf
Copy link
Member

In this PR:

@lintool before merging this, could you help with sanity checking on a different machine if it's not too much trouble? I re-ran indexing + retrieval twice on tuna to check and there were no changes to the regression numbers, but non-determinism issues might be easier to slip by...

@emmileaf emmileaf requested a review from lintool August 12, 2019 19:10
@matthew-z
Copy link
Contributor

I also added that snippet in my local, and the results are now deterministic

@lintool
Copy link
Member

lintool commented Aug 12, 2019

@emmileaf can you update the regression numbers and I'll also see if they match? Thanks!

@emmileaf
Copy link
Member Author

The regression numbers didn't change when I re-ran after the fix, so hopefully it should be able to pass with the current ones?

@lintool
Copy link
Member

lintool commented Aug 13, 2019

On tuna, I'm still getting:

2019-08-12 19:27:45,991 - regression_test - INFO - {"actual": 0.1582, "collection": "msmarco-passage", "expected": 0.1582, "metric": "map", "model": "bm25-tuned+prf", "topic": "[MS MARCO Passage Ranking: Dev Queries](https://github.com/microsoft/MSMARCO-Passage-Ranking)"}

and

2019-08-12 21:51:51,442 - regression_test - ERROR - !!!!!{"actual": 0.1583, "collection": "msmarco-passage", "expected": 0.1582, "metric": "map", "model": "bm25-tuned+prf", "topic": "[MS MARCO Passage Ranking: Dev Queries](https://github.com/microsoft/MSMARCO-Passage-Ranking)"}!!!!!

Across two different trials. Untuned BM25 seems to be okay, though. Any ideas?

@emmileaf
Copy link
Member Author

I can't seem to reproduce this error on tuna anymore…perhaps try with #776 instead? We both pushed a fix at the same time, but it looks like @matthew-z made a few additional small changes in that PR?

@lintool
Copy link
Member

lintool commented Aug 14, 2019

I reran a two times on two different machines and am now getting consistent results... I must have been inadvertently running old code? Merging.

@lintool lintool merged commit 2f1b665 into castorini:master Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MS MARCO passage regression errors: BM25prf gives non-deterministic results
3 participants