Skip to content

Conversation

dbort
Copy link
Contributor

@dbort dbort commented Sep 26, 2024

Summary:
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.

Differential Revision: D63476419

Copy link

pytorch-bot bot commented Sep 26, 2024

🔗 Helpful Links

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

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:

✅ No Failures

As of commit ca66a33 with merge base 905b88c (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 Sep 26, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

dbort added a commit to dbort/executorch that referenced this pull request Sep 27, 2024
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
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.

Reviewed By: swolchok

Differential Revision: D63476419
dbort added a commit to dbort/executorch that referenced this pull request Sep 30, 2024
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: D63476419

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 5605954.

@dbort
Copy link
Contributor Author

dbort commented Sep 30, 2024

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

@dbort dbort deleted the export-D63476419 branch September 30, 2024 21:51
pytorchbot pushed a commit that referenced this pull request Sep 30, 2024
Summary:
Pull Request resolved: #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.

Reviewed By: swolchok

Differential Revision: D63476419

fbshipit-source-id: 2300fc1fa5d5af6f6f747d3a0c31db5099389dcc
(cherry picked from commit 5605954)
@pytorchbot
Copy link
Collaborator

Cherry picking #5709

The cherry pick PR is at #5772 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

jackzhxng pushed a commit that referenced this pull request Sep 30, 2024
Fix unqualified uses of executorch functions (#5709)

Summary:
Pull Request resolved: #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.

Reviewed By: swolchok

Differential Revision: D63476419

fbshipit-source-id: 2300fc1fa5d5af6f6f747d3a0c31db5099389dcc
(cherry picked from commit 5605954)

Co-authored-by: Dave Bort <dbort@meta.com>
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.

4 participants