Skip to content

Conversation

dbort
Copy link
Contributor

@dbort dbort commented Sep 23, 2024

Summary:
Move these types out of the old torch::executor namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in torch::executor.

Most users should not use these directly, instead using the executorch::aten aliases. These ETensor types live under executorch::runtime... to make it a little more clear that they're semi-internal.

Also:

  • Hide bfloat16's bits_from_f32, which was only used by tests
  • Hide the min implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in torch::executor.

Differential Revision: D63294217

Copy link

pytorch-bot bot commented Sep 23, 2024

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ You can merge normally! (1 Unrelated Failure)

As of commit bda0a3c with merge base 905b88c (image):

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

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

@facebook-github-bot
Copy link
Contributor

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

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

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

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

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

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

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Differential Revision: D63294217
dbort added a commit to dbort/executorch that referenced this pull request Sep 27, 2024
Summary:
Pull Request resolved: pytorch#5569

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

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

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

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

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

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

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

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

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

Differential Revision: D63294217
Summary:
Pull Request resolved: pytorch#5709

I'm not sure how this worked before, but these sites called functions under torch::executor without actually qualifying them. Qualify them explicitly, because the "can call without qualification" magic stops working when we move the etensor types in D63294217.

In a few places I used `namespace etrt = executorch::runtime;` instead of a using statement for a particular function, like `etrt::isIntegralType`. If I just say `using executorch::runtime::isIntegralType`, those files fail in aten mode because the unqualified call to `isIntegralType()` is deemed ambiguous in the presence of `c10::isIntegralType()` -- but afaict that `c10` version isn't `using`'d into the global namespace, so I don't know why it conflicts. It'd be good to figure that out at some point, but this works for now.

I also updated custom_kernel_example to stop using the `torch::` namespace.

Differential Revision: D63476419

Reviewed By: swolchok
@facebook-github-bot
Copy link
Contributor

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

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

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

Differential Revision: D63294217
Summary:
Pull Request resolved: pytorch#5569

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 6ff52cc.

@dbort dbort deleted the export-D63294217 branch September 30, 2024 21:51
@dbort
Copy link
Contributor Author

dbort commented Sep 30, 2024

@pytorchbot cherry-pick --onto release/0.4 -c fixnewfeature

pytorchbot pushed a commit that referenced this pull request Sep 30, 2024
Summary:
Pull Request resolved: #5569

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

Differential Revision: D63294217

fbshipit-source-id: 5241fd5a0b487631b6460222b6c3e27e82aea5f0
(cherry picked from commit 6ff52cc)
@pytorchbot
Copy link
Collaborator

Cherry picking #5569

The cherry pick PR is at #5773 and it is recommended to link a fixnewfeature cherry pick PR with an issue. The following tracker issues are updated:

Details for Dev Infra team Raised by workflow job

dbort added a commit that referenced this pull request Oct 1, 2024
Summary:
Pull Request resolved: #5569

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

Differential Revision: D63294217

fbshipit-source-id: 5241fd5a0b487631b6460222b6c3e27e82aea5f0
(cherry picked from commit 6ff52cc)
dbort added a commit that referenced this pull request Oct 3, 2024
Move etensor types to their new namespace (#5569)

Summary:
Pull Request resolved: #5569

Move these types out of the old `torch::executor` namespace. Put them in their own namespace so that no-one will see them by default, unlike their old location in `torch::executor`.

Most users should not use these directly, instead using the `executorch::aten` aliases. These ETensor types live under `executorch::runtime...` to make it a little more clear that they're semi-internal.

Also:
- Hide bfloat16's `bits_from_f32`, which was only used by tests
- Hide the `min` implementation used by string_view. Although it was in an anonymous namespace, it was in a header, so it was visible to all code in `torch::executor`.
- Remove an unnecessary forward declaration from BlasKernel.h which conflicted with the new namespace

Reviewed By: swolchok

Differential Revision: D63294217

fbshipit-source-id: 5241fd5a0b487631b6460222b6c3e27e82aea5f0
(cherry picked from commit 6ff52cc)

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.

4 participants