Skip to content

Conversation

@helunwencser
Copy link
Contributor

@helunwencser helunwencser commented Oct 7, 2024

Summary:
This PR fixes a bunch of issues in the eval pipeline:

  • Use the right token for eot_token_id
  • Do not add bos and eos during tok_encode based on this discussion.
  • Update executorch/examples/models/llama2/tokenizer/tiktoken.py to be synced with llama 3.1's official code. Majorly updated set of special tokens.

Differential Revision: D62198560

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 7, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 4158d4c with merge base 36a5bc6 (image):
💚 Looks good so far! There are no failures yet. 💚

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 Oct 7, 2024
@facebook-github-bot
Copy link
Contributor

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

helunwencser pushed a commit to helunwencser/executorch that referenced this pull request Oct 7, 2024
Summary:

This PR fixes a bunch of issues in the eval pipeline:
- Use the right token for `eot_token_id`
- Do not add `bos` and `eos` during `tok_encode` based on this [discussion](https://fburl.com/code/uifmt746).
- Update `executorch/examples/models/llama2/tokenizer/tiktoken.py` to be synced with llama 3.1's official [code](https://github.com/meta-llama/llama-models/blob/main/models/llama3/api/tokenizer.py). Majorly updated set of special tokens.
- Update `--limit`'s default value to none based on `lm_eval`'s [doc](https://github.com/EleutherAI/lm-evaluation-harness/blob/main/docs/interface.md). It should only be used during test.
- Update `--dtype-override`'s default value to none such that we don't override mode's dtype by default.

For the context, we observed a gap between ExecuTorch's eval result and SpinQuant's eval result: 19 vs 14.

After these fixes, ExecuTorch's eval result is closer to SpinQuant's eval.

Differential Revision: D62198560
@facebook-github-bot
Copy link
Contributor

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

helunwencser pushed a commit to helunwencser/executorch that referenced this pull request Oct 8, 2024
Summary:

This PR fixes a bunch of issues in the eval pipeline:
- Use the right token for `eot_token_id`
- Do not add `bos` and `eos` during `tok_encode` based on this [discussion](https://fburl.com/code/uifmt746).
- Update `executorch/examples/models/llama2/tokenizer/tiktoken.py` to be synced with llama 3.1's official [code](https://github.com/meta-llama/llama-models/blob/main/models/llama3/api/tokenizer.py). Majorly updated set of special tokens.
- Update `--limit`'s default value to none based on `lm_eval`'s [doc](https://github.com/EleutherAI/lm-evaluation-harness/blob/main/docs/interface.md). It should only be used during test.
- Update `--dtype-override`'s default value to none such that we don't override mode's dtype by default.

For the context, we observed a gap between ExecuTorch's eval result and SpinQuant's eval result: 19 vs 14.

After these fixes, ExecuTorch's eval result is closer to SpinQuant's eval.

Differential Revision: D62198560
@facebook-github-bot
Copy link
Contributor

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

helunwencser pushed a commit to helunwencser/executorch that referenced this pull request Oct 8, 2024
Summary:

This PR fixes a bunch of issues in the eval pipeline:
- Use the right token for `eot_token_id`
- Do not add `bos` and `eos` during `tok_encode` based on this [discussion](https://fburl.com/code/uifmt746).
- Update `executorch/examples/models/llama2/tokenizer/tiktoken.py` to be synced with llama 3.1's official [code](https://github.com/meta-llama/llama-models/blob/main/models/llama3/api/tokenizer.py). Majorly updated set of special tokens.

Differential Revision: D62198560
@facebook-github-bot
Copy link
Contributor

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

@mergennachin mergennachin self-requested a review October 8, 2024 23:46
Summary:

This PR fixes a bunch of issues in the eval pipeline:
- Use the right token for `eot_token_id`
- Do not add `bos` and `eos` during `tok_encode` based on this [discussion](https://fburl.com/code/uifmt746).
- Update `executorch/examples/models/llama2/tokenizer/tiktoken.py` to be synced with llama 3.1's official [code](https://github.com/meta-llama/llama-models/blob/main/models/llama3/api/tokenizer.py). Majorly updated set of special tokens.

Reviewed By: mergennachin

Differential Revision: D62198560
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 2027a14.

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