Skip to content

Error out when token is outside of vocab size#3535

Closed
cccclai wants to merge 1 commit intopytorch:mainfrom
cccclai:export-D57057026
Closed

Error out when token is outside of vocab size#3535
cccclai wants to merge 1 commit intopytorch:mainfrom
cccclai:export-D57057026

Conversation

@cccclai
Copy link
Contributor

@cccclai cccclai commented May 7, 2024

Summary:
Ideally it shouldn't happen, but if we post process the weight somehow too much it might happen. In Android, it just seg fault directly if it's outside of the range without error message. After this change, it's clearer:

I 00:00:00.180837 executorch:runner.cpp:364] Finish getting logits_tensor...
I 00:00:00.180907 executorch:runner.cpp:395] Start running tokenizer_->decode...
E 00:00:00.180911 executorch:bpe_tokenizer.cpp:155] token 18446744073709551615 is out side of vacab range 512
I 00:00:00.180914 executorch:runner.cpp:397] Finish running tokenizer_->decode...
F 00:00:00.180918 executorch:runner.cpp:398] In function generate(), assert failed: piece_res.ok()
Aborted

Differential Revision: D57057026

@pytorch-bot
Copy link

pytorch-bot bot commented May 7, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/3535

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures

As of commit 00d9e60 with merge base f9db02a (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 7, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57057026

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57057026

cccclai added a commit to cccclai/executorch-1 that referenced this pull request May 7, 2024
Summary:

Ideally it shouldn't happen, but if we post process the weight somehow too much it might happen. In Android, it just seg fault directly if it's outside of the range without error message. After this change, it's clearer:
```
E 00:00:00.180911 executorch:bpe_tokenizer.cpp:155] token 18446744073709551615 is out side of vacab range 512
Aborted
```

Differential Revision: D57057026
@cccclai cccclai force-pushed the export-D57057026 branch from 8158451 to a178295 Compare May 7, 2024 22:03
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57057026

Summary:

Ideally it shouldn't happen, but if we post process the weight somehow too much it might happen. In Android, it just seg fault directly if it's outside of the range without error message. After this change, it's clearer:
```
E 00:00:00.180911 executorch:bpe_tokenizer.cpp:155] token 18446744073709551615 is out side of vacab range 512
Aborted
```

Reviewed By: larryliu0820

Differential Revision: D57057026
@cccclai cccclai force-pushed the export-D57057026 branch from a178295 to 00d9e60 Compare May 8, 2024 17:38
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57057026

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in f8403ed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants