Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upUpdate trec_eval Fix #475 #477
Conversation
This comment has been minimized.
This comment has been minimized.
Hi @Chriskamphuis I only see removal of the existing tarball? There isn't a new one? |
This comment has been minimized.
This comment has been minimized.
Hi @lintool, I replaced it with a tarball with the same name. The commit shows that the file is modified (not deleted). |
This comment has been minimized.
This comment has been minimized.
Ah, I see - misread. The new tarball is created from https://github.com/usnistgov/trec_eval/ ? |
This comment has been minimized.
This comment has been minimized.
I included the commit id in the commit that changed the name, the commit id of usnistgov/trec_eval is a5211566d0c9e2ec337bacf327b9350ab5b3edde. |
This comment has been minimized.
This comment has been minimized.
@Chriskamphuis sorry to be kinda a pain, but we're try to implement rigorous software engineering practices in this project - CI is failing because of this - https://github.com/castorini/Anserini/blob/master/.travis.yml#L16 File name needs to be updated. Can you please do so? |
This comment has been minimized.
This comment has been minimized.
@lintool Yes, no problem, I am working on it. I will also update the markdown files so it will be consistent. |
Peilin-Yang
reviewed
Nov 16, 2018
@@ -60,47 +60,47 @@ nohup target/appassembler/bin/SearchCollection -topicreader Webxml -index lucene | |||
Evaluation can be performed using `trec_eval` and `gdeval.pl`: | |||
|
|||
``` | |||
eval/trec_eval.9.0/trec_eval -m map -m P.30 src/main/resources/topics-and-qrels/qrels.web.51-100.txt run.cw09b.bm25.topics.web.51-100.txt |
This comment has been minimized.
This comment has been minimized.
Peilin-Yang
Nov 16, 2018
Collaborator
It is a bit odd why there is an extra empty line here.
Did you manually change this file?
This comment has been minimized.
This comment has been minimized.
lintool
Nov 16, 2018
Member
@Peilin-Yang are those parts auto-gen'ed? Perhaps @Chriskamphuis doesn't know about the details of the regression framework...
This comment has been minimized.
This comment has been minimized.
Peilin-Yang
Nov 16, 2018
Collaborator
All experiments-{$collection}.md
files are auto-generated.
@Chriskamphuis Could you please run mvn test
locally and see if there is any diff?
This comment has been minimized.
This comment has been minimized.
Chriskamphuis
Nov 16, 2018
Author
Contributor
@Peilin-Yang No diff for me locally. I only changed the the index.md and yaml files. Then I ran test before pushing the changes. Maybe I accidentally added an extra line when checking if everything was correct, but then it should be removed when running mvn test
right?
Peilin-Yang
approved these changes
Nov 16, 2018
lintool
merged commit b4064da
into
castorini:master
Nov 16, 2018
1 check passed
This comment has been minimized.
This comment has been minimized.
@Chriskamphuis thanks for your first contribution. Looking forward to many more in the future! |
Chriskamphuis commentedNov 16, 2018
Updated trec_eval to latest version. Running old version can result in a
segmentation fault.