Skip to content

Move examples/models/... out of the torch namespace #5318

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

Closed
wants to merge 1 commit into from

Conversation

dbort
Copy link
Contributor

@dbort dbort commented Sep 12, 2024

Summary:
The code under examples/... is a proxy for user code, and users should never declare code under the torch:: or executorch:: namespaces.

Move this code under the example:: namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use enum class to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Differential Revision: D62591348

Copy link

pytorch-bot bot commented Sep 12, 2024

🔗 Helpful Links

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

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

❌ 5 New Failures, 1 Cancelled Job, 1 Unrelated Failure

As of commit 39b248c with merge base 67be84b (image):

NEW FAILURES - The following jobs have failed:

CANCELLED JOB - The following job was cancelled. Please retry:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

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

@facebook-github-bot
Copy link
Contributor

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

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

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 13, 2024
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

Differential Revision: D62591348
Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Tests:
Should be a no-op; CI passes.

Prints no output:
```
find examples/models -type f | xargs grep "namespace torch"
find examples/models -type f | xargs grep "namespace executorch"
```

Build llava_main:
```
bash .ci/scripts/test_llava.sh
```

Reviewed By: larryliu0820

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 262dfc0.

dbort added a commit to dbort/executorch that referenced this pull request Sep 20, 2024
Fix the build errors I introduced in pytorch#5318

Test Plan:
Build the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md
@dbort dbort mentioned this pull request Sep 20, 2024
dbort added a commit to dbort/executorch that referenced this pull request Sep 20, 2024
Fix the build errors I introduced in pytorch#5318

Test Plan:
Build the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md
facebook-github-bot pushed a commit that referenced this pull request Sep 20, 2024
Summary:
Fix the build errors I introduced in #5318

Pull Request resolved: #5513

Test Plan: Built the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

Reviewed By: helunwencser

Differential Revision: D63133231

Pulled By: dbort

fbshipit-source-id: 81d5402e76fd86c919a13a1b1b83687feca72ab7
pytorchbot pushed a commit that referenced this pull request Sep 20, 2024
Summary:
Fix the build errors I introduced in #5318

Pull Request resolved: #5513

Test Plan: Built the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

Reviewed By: helunwencser

Differential Revision: D63133231

Pulled By: dbort

fbshipit-source-id: 81d5402e76fd86c919a13a1b1b83687feca72ab7
(cherry picked from commit 8618607)
@pytorchbot pytorchbot mentioned this pull request Sep 20, 2024
jackzhxng pushed a commit that referenced this pull request Sep 22, 2024
Fix phi-3-mini build (#5513)

Summary:
Fix the build errors I introduced in #5318

Pull Request resolved: #5513

Test Plan: Built the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

Reviewed By: helunwencser

Differential Revision: D63133231

Pulled By: dbort

fbshipit-source-id: 81d5402e76fd86c919a13a1b1b83687feca72ab7
(cherry picked from commit 8618607)

Co-authored-by: Dave Bort <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk 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