Skip to content

[ExecuTorch][#10364] Add Protected Method Getter in extension.Module #10374

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

Merged

Conversation

zhenyan-zhang-meta
Copy link
Contributor

@zhenyan-zhang-meta zhenyan-zhang-meta commented Apr 22, 2025

Stack from ghstack (oldest at bottom):

Context

This issue is a step of #9638. In the discussion, we want to unblock having extension/Module as the single source of implementation, which means that pybindings/PyModule should use extension/Module rather than its own.

Although we are decouping method getter from pybindings implementation, method getter itself is still needed. To keep having the method getter while not exposing it, we can create a protected method getter and confine it's usage inside child classes that we are about to create.

Proposal

Add a protected get_method to extension.Module, taking method name string as an input.

Differential Revision: D73473766

…dule

# Context

This issue is a step of #9638. In the discussion, we want to unblock having `extension/Module` as the single source of implementation, which means that `pybindings/PyModule` should use `extension/Module` rather than its own.

Although we are decouping method getter from `pybindings` implementation, method getter itself is still needed. To keep having the method getter while not exposing it, we can create a protected method getter and confine it's usage inside child classes that we are about to create.

# Proposal

Add a protected `get_method` to `extension.Module`, taking method name string as an input.

Differential Revision: [D73473766](https://our.internmc.facebook.com/intern/diff/D73473766/)

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Apr 22, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit daf2a80 with merge base 95c663e (image):
💚 Looks good so far! There are no failures yet. 💚

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

zhenyan-zhang-meta pushed a commit that referenced this pull request Apr 22, 2025
…dule

# Context

This issue is a step of #9638. In the discussion, we want to unblock having `extension/Module` as the single source of implementation, which means that `pybindings/PyModule` should use `extension/Module` rather than its own.

Although we are decouping method getter from `pybindings` implementation, method getter itself is still needed. To keep having the method getter while not exposing it, we can create a protected method getter and confine it's usage inside child classes that we are about to create.

# Proposal

Add a protected `get_method` to `extension.Module`, taking method name string as an input.

Differential Revision: [D73473766](https://our.internmc.facebook.com/intern/diff/D73473766/)

ghstack-source-id: 279669340
Pull Request resolved: #10374
@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 Apr 22, 2025
@zhenyan-zhang-meta zhenyan-zhang-meta self-assigned this Apr 22, 2025
@facebook-github-bot
Copy link
Contributor

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

@zhenyan-zhang-meta
Copy link
Contributor Author

#10364

Copy link
Contributor

@Gasoonjia Gasoonjia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@facebook-github-bot facebook-github-bot merged commit d7dd048 into gh/zhenyan-zhang-meta/4/base Apr 23, 2025
92 of 96 checks passed
@facebook-github-bot facebook-github-bot deleted the gh/zhenyan-zhang-meta/4/head branch April 23, 2025 09:36
@zhenyan-zhang-meta zhenyan-zhang-meta changed the title [ExecuTorch][#9638] Introduce Protected Method Getter in Extension.Module [ExecuTorch][#10364] Introduce protected method getter in extension.Module Apr 23, 2025
@zhenyan-zhang-meta zhenyan-zhang-meta changed the title [ExecuTorch][#10364] Introduce protected method getter in extension.Module [ExecuTorch][#10364] Add Protected Method Getter in extension.Module Apr 23, 2025
jackzhxng pushed a commit that referenced this pull request Apr 24, 2025
…dule (#10384)

This PR was created by the merge bot to help merge the original PR into
the main branch.
ghstack PR number: #10374 by
@zhenyan-zhang-meta
^ Please use this as the source of truth for the PR details, comments,
and reviews
ghstack PR base:
https://github.com/pytorch/executorch/tree/gh/zhenyan-zhang-meta/4/base
ghstack PR head:
https://github.com/pytorch/executorch/tree/gh/zhenyan-zhang-meta/4/head
Merge bot PR base: https://github.com/pytorch/executorch/tree/main
Merge bot PR head:
https://github.com/pytorch/executorch/tree/gh/zhenyan-zhang-meta/4/orig
@diff-train-skip-merge

Co-authored-by: zhenyanzhang <[email protected]>
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. topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants