Skip to content

Conversation

why-in-Shanghaitech
Copy link
Contributor

@why-in-Shanghaitech why-in-Shanghaitech commented Aug 28, 2024

This PR resolves #134408. Add an additional test and have passed the local test.

Do you think we should add a post-check to ensure args and kwargs are not both None? It seems to be possible to have modules without inputs.

This PR does not include any such post-check.

cc @albanD @mruberry @jbschlosser @walterddr @mikaylagawarecki

Copy link

pytorch-bot bot commented Aug 28, 2024

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 0acaa6a with merge base 356f14e (image):

NEW FAILURE - The following job has failed:

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

Copy link

linux-foundation-easycla bot commented Aug 28, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: why-in-Shanghaitech / name: Haoyi Wu (0acaa6a)

@janeyx99 janeyx99 requested a review from zou3519 August 30, 2024 04:49
@janeyx99 janeyx99 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 30, 2024
@zou3519
Copy link
Contributor

zou3519 commented Aug 30, 2024

Do you think we should add a post-check to ensure args and kwargs are not both None? It seems to be possible to have modules without inputs.

No, the module might not have any inputs.

@zou3519 zou3519 added module: nn Related to torch.nn release notes: nn release notes category labels Aug 30, 2024
@why-in-Shanghaitech
Copy link
Contributor Author

When will this merge? I'm wondering whether it will appear in the next release. @zou3519

@zou3519
Copy link
Contributor

zou3519 commented Sep 11, 2024

@pytorchbot rebase

@zou3519 zou3519 added the suppress-bc-linter Suppresses the failures of API backward-compatibility linter (Lint/bc_linter) label Sep 11, 2024
@zou3519
Copy link
Contributor

zou3519 commented Sep 11, 2024

BC lint seems erroneous, we are making a mandatory argument optional

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased my-nightly-branch onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout my-nightly-branch && git pull --rebase)

@zou3519
Copy link
Contributor

zou3519 commented Sep 11, 2024

@why-in-Shanghaitech if/when the tests pass, you can comment "@pytorchbot merge" and it'll merge the PR. Or I can do that, just comment on this thread again if you don't see it being merged

@zou3519 zou3519 added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 11, 2024
@why-in-Shanghaitech
Copy link
Contributor Author

Oops... it seems that we have a time-out error...

@zou3519
Copy link
Contributor

zou3519 commented Sep 12, 2024

@pytorchbot merge -f "unrelated error"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Sep 20, 2024
…ytorch#134643)

This PR resolves pytorch#134408. Add an additional test and have passed the local test.

Do you think we should add a post-check to ensure `args` and `kwargs` are not both `None`? It seems to be possible to have modules without inputs.

This PR does not include any such post-check.

Pull Request resolved: pytorch#134643
Approved by: https://github.com/zou3519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: nn Related to torch.nn open source release notes: nn release notes category suppress-bc-linter Suppresses the failures of API backward-compatibility linter (Lint/bc_linter) triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the arguments in torch.func.functional_call optional
5 participants