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

[Reland] Remove calls of c10::either #109708

Closed
wants to merge 1 commit into from

Conversation

cyyever
Copy link
Collaborator

@cyyever cyyever commented Sep 20, 2023

While there were FB issues encountered when removing c10::either #109299 , we should be able to change OSS code.

cc @clee2000

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 20, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit 65f589c with merge base b30ee35 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added the release notes: jit release notes category label Sep 20, 2023
@cyyever
Copy link
Collaborator Author

cyyever commented Sep 20, 2023

@pytorchmergebot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 20, 2023
@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

@clee2000
Copy link
Contributor

I think this broke test_cpp_extensions_aot_no_ninja on windows cuda. Error looks like https://hud.pytorch.org/pytorch/pytorch/commit/e4d8ec9fe82b6186efbff0e2a7b8393b157ef298

@atalman
Copy link
Contributor

atalman commented Sep 21, 2023

HI @clee2000 I agree I am looking at this error now. Error on Line 626 refers to this PR:

2023-09-21T03:27:44.3370881Z C:/actions-runner/_work/pytorch/pytorch/build/win_tmp/build/torch/include\torch/library.h(626): error C2872: 'std': ambiguous symbol
2023-09-21T03:27:44.3371551Z C:/Program Files (x86)/Microsoft Visual Studio/2019/BuildTools/VC/Tools/MSVC/14.29.30133/include\valarray(21): note: could be 'std'
2023-09-21T03:27:44.3372238Z C:/actions-runner/_work/pytorch/pytorch/build/win_tmp/build/torch/include\torch/library.h(626): note: or       'std'
2023-09-21T03:27:44.3373041Z torch_library.cu(6): note: see reference to function template instantiation 'torch::Library &torch::Library::def<const char(&)[12],bool(__cdecl *)(bool,bool)>(NameOrSchema,Func &&) &' being compiled
2023-09-21T03:27:44.3373573Z         with
2023-09-21T03:27:44.3374668Z         [
2023-09-21T03:27:44.3374930Z             NameOrSchema=const char (&)[12],
2023-09-21T03:27:44.3375237Z             Func=bool (__cdecl *)(bool,bool)
2023-09-21T03:27:44.3375433Z         ]
Screenshot 2023-09-21 at 3 54 12 PM

@atalman
Copy link
Contributor

atalman commented Sep 21, 2023

@pytorchbot revert -m "Broke windows periodic tests" -c nosignal

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

@cyyever your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Sep 21, 2023
@cyyever
Copy link
Collaborator Author

cyyever commented Sep 22, 2023

@clee2000 It was likely a MSVC bug

@cyyever cyyever deleted the remove_either_reland branch January 31, 2024 00:38
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 release notes: jit release notes category Reverted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants