-
Notifications
You must be signed in to change notification settings - Fork 364
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
LTR document searcher and docs to reproduce #835
Conversation
stephaniewhoo
commented
Oct 26, 2021
- add document scripts
- add document searcher
- add document repro doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! Successfully get it reproduced in orca.
@@ -0,0 +1,217 @@ | |||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of code duplicate here with: https://github.com/castorini/pyserini/blob/master/pyserini/ltr/search_msmarco_passage/__main__.py
|
||
# We're going to explicitly use a local installation of Pyserini (as opposed to a pip-installed one). | ||
# Comment these lines out to use a pip-installed one instead. | ||
sys.path.insert(0, './') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this really suggests that this script should be in scripts/
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For that matter, shouldn't pyserini/ltr/search_msmarco_passage/__main__.py
be in scripts/
also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we reorganize the LTR pipeline according to #611, in order to have something like python -m pyserini.ltr.search_msmarco_passage \
If the main goes to scripts/, it will become sth python xxx.py
. Is this what you mean?
They have many duplicate codes since the document exp uses passage-like segments in LTR process. Or I change codes in pyserini/ltr/search_msmarco_passage/
to make it work for both passage/segments in doc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be python xxx.py
.
The reason is that python -m ...
should work with just a pip install... but since you are depending on a checkout, python -m ...
wouldn't work anyways, right?
close as new PR #844 |