8000 Fix tokenizer length issue with DJL upgrade by lintool · Pull Request #2536 · castorini/anserini · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix tokenizer length issue with DJL upgrade #2536

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 21, 2024
Merged

Fix tokenizer length issue with DJL upgrade #2536

merged 1 commit into from
Jun 21, 2024

Conversation

lintool
Copy link
Member
@lintool lintool commented Jun 21, 2024

Another take for #2535

After digging through source code:
https://github.com/deepjavalibrary/djl/blob/master/extensions/tokenizers/src/main/java/ai/djl/huggingface/tokenizers/HuggingFaceTokenizer.java

I was able to override the tokenize arbitrarily long sequences.

theyorubayesian
theyorubayesian approved these changes Jun 21, 2024
View reviewed changes
Copy link
Member
@theyorubayesian theyorubayesian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this looks good to me

@codecov Codecov
Copy link
codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.18%. Comparing base (98e4866) to head (6185321).

Files Patch % Lines
...n/java/io/anserini/analysis/CompositeAnalyzer.java 66.66% 0 Missing and 1 partial ⚠️
...nserini/analysis/HuggingFaceTokenizerAnalyzer.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #2536      +/-   ##
============================================
+ Coverage     67.14%   67.18%   +0.03%     
- Complexity     1479     1481       +2     
============================================
  Files           219      219              
  Lines         12641    12645       +4     
  Branches       1528     1528              
============================================
+ Hits           8488     8495       +7     
+ Misses         3625     3624       -1     
+ Partials        528      526       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ToluClassics
Copy link
Member

This works, alternatively we could just set the truncation flag to "DO NOT TRUNCATE"

options.put("truncation", "DO_NOT_TRUNCATE");

Ref: https://github.com/deepjavalibrary/djl/blob/cac16a1918af9b033b62c2bd51acd0f1cabd8a30/extensions/tokenizers/src/main/java/ai/djl/huggingface/tokenizers/HuggingFaceTokenizer.java#L556

@lintool
Copy link
Member Author
lintool commented Jun 21, 2024

I've confirmed that all these affected regressions pass:

python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-ar-aca > logs/log.miracl-v1.0-ar-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-bn-aca > logs/log.miracl-v1.0-bn-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-en-aca > logs/log.miracl-v1.0-en-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-es-aca > logs/log.miracl-v1.0-es-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-fa-aca > logs/log.miracl-v1.0-fa-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-fi-aca > logs/log.miracl-v1.0-fi-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-fr-aca > logs/log.miracl-v1.0-fr-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-hi-aca > logs/log.miracl-v1.0-hi-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-id-aca > logs/log.miracl-v1.0-id-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-ja-aca > logs/log.miracl-v1.0-ja-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-ko-aca > logs/log.miracl-v1.0-ko-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-ru-aca > logs/log.miracl-v1.0-ru-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-sw-aca > logs/log.miracl-v1.0-sw-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-te-aca > logs/log.miracl-v1.0-te-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-th-aca > logs/log.miracl-v1.0-th-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression miracl-v1.0-zh-aca > logs/log.miracl-v1.0-zh-aca.txt 2>&1

python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-ar-aca > logs/log.mrtydi-v1.1-ar-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-bn-aca > logs/log.mrtydi-v1.1-bn-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-en-aca > logs/log.mrtydi-v1.1-en-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-fi-aca > logs/log.mrtydi-v1.1-fi-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-id-aca > logs/log.mrtydi-v1.1-id-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-ja-aca > logs/log.mrtydi-v1.1-ja-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-ko-aca > logs/log.mrtydi-v1.1-ko-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-ru-aca > logs/log.mrtydi-v1.1-ru-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-sw-aca > logs/log.mrtydi-v1.1-sw-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-te-aca > logs/log.mrtydi-v1.1-te-aca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression mrtydi-v1.1-th-aca > logs/log.mrtydi-v1.1-th-aca.txt 2>&1

python src/main/python/run_regression.py --index --verify --search --regression msmarco-v1-passage.wp-ca > logs/log.msmarco-v1-passage.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression msmarco-v1-doc.wp-ca > logs/log.msmarco-v1-doc.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression msmarco-v1-doc-segmented.wp-ca > logs/log.msmarco-v1-doc-segmented.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl19-passage.wp-ca > logs/log.dl19-passage.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl19-doc.wp-ca > logs/log.dl19-doc.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl19-doc-segmented.wp-ca > logs/log.dl19-doc-segmented.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl20-passage.wp-ca > logs/log.dl20-passage.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl20-doc.wp-ca > logs/log.dl20-doc.wp-ca.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl20-doc-segmented.wp-ca > logs/log.dl20-doc-segmented.wp-ca.txt 2>&1

python src/main/python/run_regression.py --index --verify --search --regression msmarco-v1-passage.wp-hgf > logs/log.msmarco-v1-passage.wp-hgf.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression msmarco-v1-doc.wp-hgf > logs/log.msmarco-v1-doc.wp-hgf.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl19-passage.wp-hgf > logs/log.dl19-passage.wp-hgf.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl19-doc.wp-hgf > logs/log.dl19-doc.wp-hgf.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl20-passage.wp-hgf > logs/log.dl20-passage.wp-hgf.txt 2>&1
python src/main/python/run_regression.py --index --verify --search --regression dl20-doc.wp-hgf > logs/log.dl20-doc.wp-hgf.txt 2>&1

@lintool lintool merged commit afa6ab0 into master Jun 21, 2024
2 of 3 checks passed
@lintool lintool deleted the djl-upgrade2 branch June 21, 2024 21:32
@lintool lintool mentioned this pull request Jun 21, 2024
Closed
Panizghi pushed a commit to Panizghi/anserini that referenced this pull request Jul 9, 2024
@lintool @Panizghi
Fix tokenizer length issue with DJL upgrade (castorini#2536)
7224c20 
Override default settings to be able to tokenize arbitrarily long sequences.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theyorubayesian theyorubayesian theyorubayesian approved these changes

@ToluClassics ToluClassics Awaiting requested review from ToluClassics

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lintool @ToluClassics @theyorubayesian
0