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

Explicit line endings and encoding for MSMARCO passage on Windows #953

Merged
merged 1 commit into from
Jan 15, 2020
Merged

Explicit line endings and encoding for MSMARCO passage on Windows #953

merged 1 commit into from
Jan 15, 2020

Conversation

edwinzhng
Copy link
Member

Line endings + encoding errors in Python prevented the MSMARCO passage experiments from being replicated on Windows so this should fix it. I also re-ran the experiment again on Linux and it still worked.

Will wait for @LuKuuu to also confirm that it works for him on Windows

@codecov
Copy link

codecov bot commented Jan 15, 2020

Codecov Report

Merging #953 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #953   +/-   ##
=========================================
  Coverage     39.56%   39.56%           
  Complexity      516      516           
=========================================
  Files           120      120           
  Lines          7231     7231           
  Branches       1086     1086           
=========================================
  Hits           2861     2861           
  Misses         4083     4083           
  Partials        287      287

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51879e0...e5b436d. Read the comment docs.

@lintool
Copy link
Member

lintool commented Jan 15, 2020

I'll wait for +1 from @LuKuuu before I ship.

@LuKuuu
Copy link
Contributor

LuKuuu commented Jan 15, 2020

@lintool I just tested it and it works on my device.

@lintool lintool merged commit f21137b into castorini:master Jan 15, 2020
@lintool
Copy link
Member

lintool commented Jan 15, 2020

@LuKuuu can you add in your own replication record, per #939

Thanks!

crystina-z pushed a commit to crystina-z/anserini that referenced this pull request Oct 28, 2022
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.

3 participants