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

Enable UFMT on test/test_functionalization.py #123926

Closed
wants to merge 4 commits into from
Closed

Conversation

huniu20
Copy link
Contributor

@huniu20 huniu20 commented Apr 12, 2024

Part of #123062
cc @ezyang

Copy link

pytorch-bot bot commented Apr 12, 2024

🔗 Helpful Links

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

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

❌ 3 New Failures

As of commit d02183a with merge base 9dfeec9 (image):

NEW FAILURES - The following jobs have failed:

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

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Apr 12, 2024
@huniu20 huniu20 changed the title Enable EFMT on test/test_functionalization.py Enable UFMT on test/test_functionalization.py Apr 12, 2024
@ezyang
Copy link
Contributor

ezyang commented Apr 12, 2024

clean

diff --git b/test/test_functionalization.py a/test/test_functionalization.py
index 3c1f3c823f2..e79ab0910ef 100644
--- b/test/test_functionalization.py
+++ a/test/test_functionalization.py
@@ -81,6 +81,7 @@ def _functionalize(
     TEST_WITH_TORCHDYNAMO, "https://github.com/pytorch/pytorch/issues/81457"
 )
 class TestFunctionalization(TestCase):
+
     crossref = False
 
     def get_logs(self, func, *inpts, reapply_views=False, run_reinplace=False):

@ezyang
Copy link
Contributor

ezyang commented Apr 12, 2024

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 12, 2024
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@ezyang
Copy link
Contributor

ezyang commented Apr 13, 2024

needs some more whackin

@ezyang
Copy link
Contributor

ezyang commented Apr 14, 2024

clean

@ezyang
Copy link
Contributor

ezyang commented Apr 14, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@huniu20
Copy link
Contributor Author

huniu20 commented Apr 14, 2024

@ezyang Hello, Yang, thank you for reviewing this PR. I don't know how to fix this failure of CI, I would be very grateful if you could give me some advice.

@ezyang
Copy link
Contributor

ezyang commented Apr 15, 2024

you need to move the noflake pragmas so that they are on the quoted string line, black moved them to the wrong line

Copy link
Contributor

@statelesshz statelesshz left a comment

Choose a reason for hiding this comment

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

for reference only :)

@@ -217,7 +268,8 @@ def forward(self, arg0_1):
view_copy_11 = torch.ops.aten.view_copy.default(view_copy_8, [16, 64, 128, 128]); view_copy_8 = None
detach_copy_1 = torch.ops.aten.detach_copy.default(view_copy_11); view_copy_11 = None
return detach_copy_1
""") # noqa: B950
""",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
""",
""", # noqa: B950

test/test_functionalization.py Show resolved Hide resolved
@@ -455,16 +560,20 @@ def forward(self, arg0_1):
getitem_5 = _fused_moving_avg_obs_fq_helper_functional[5]; _fused_moving_avg_obs_fq_helper_functional = None
copy_ = torch.ops.aten.copy_.default(arg0_1, getitem_5); arg0_1 = getitem_5 = None
return (getitem, getitem_1)
""") # noqa: B950
""",
) # noqa: B950
Copy link
Contributor

Choose a reason for hiding this comment

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

Move the noflake pragma so that it is on the quoted string line

@huniu20
Copy link
Contributor Author

huniu20 commented Apr 26, 2024

@ezyang Hi, sorry about the delay of fix noflake issues and thanks for your review

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 3 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@ezyang
Copy link
Contributor

ezyang commented Apr 26, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 2 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@huniu20
Copy link
Contributor Author

huniu20 commented Apr 28, 2024

@ezyang Hello, Yang, I don't know what was wrong with the failure of CI. This PR does not involve those three files:

  • test/test_proxy_tensor.py
  • test/test_torch.py
  • test/test_fx.py

BTW, those three tests all passed locally using python -m pytest test/test_xxx.py.

@huniu20
Copy link
Contributor Author

huniu20 commented Apr 28, 2024

Thanks a lot for your kind review 💯

@ezyang
Copy link
Contributor

ezyang commented Apr 28, 2024

@pytorchbot merge -f "I think these are unrelated"

@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

carmocca pushed a commit to carmocca/pytorch that referenced this pull request Apr 29, 2024
andoorve pushed a commit to andoorve/pytorch that referenced this pull request May 1, 2024
petrex pushed a commit to petrex/pytorch that referenced this pull request May 3, 2024
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 open source topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants