Skip to content
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

Make IR private #1099

Merged
merged 1 commit into from
May 2, 2024
Merged

Make IR private #1099

merged 1 commit into from
May 2, 2024

Conversation

kwen2501
Copy link
Contributor

@kwen2501 kwen2501 commented May 1, 2024

User-facing APIs in IR.py have been lifted into __init__.py. Thus it seems to make sense to make IR.py private.

Copy link
Member

@H-Huang H-Huang left a comment

Choose a reason for hiding this comment

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

Looks good! I assume this will break any users using pippy.IR right now, so should there be an error message to switch? Or does it not matter since we are migrating.

@@ -7,7 +7,7 @@

import torch
import torch.distributed as dist
from pippy.IR import pipe_split
from pippy._IR import pipe_split
Copy link
Member

Choose a reason for hiding this comment

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

nit: just pippy?

@wconstab
Copy link
Contributor

wconstab commented May 2, 2024

fine with me. although, depending on if we care about tip of PipPy breaking we could just do the rename in the pytorch side. i dont think this matters though. once we land it then there is no point of keeping PipPy repo/pkg maintained

@kwen2501 kwen2501 merged commit 6b19ff5 into main May 2, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants