Skip to content

Revert "Attempt to fix windows build"#35217

Closed
xuhdev wants to merge 1 commit intopytorch:masterfrom
xuhdev:ci-all/vs-2017-bug
Closed

Revert "Attempt to fix windows build"#35217
xuhdev wants to merge 1 commit intopytorch:masterfrom
xuhdev:ci-all/vs-2017-bug

Conversation

@xuhdev
Copy link
Copy Markdown
Collaborator

@xuhdev xuhdev commented Mar 23, 2020 •
edited
Loading

This reverts commit 0c22255.

@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Mar 23, 2020 •
edited
Loading

💊 CircleCI build failures summary and remediations

As of commit 0993da2 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no CircleCI failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 17 times.

@xuhdev xuhdev mentioned this pull request Mar 23, 2020
Closed
@xuhdev xuhdev requested review from ezyang and jamesr66a and removed request for jamesr66a March 23, 2020 18:07
@xuhdev
Copy link
Copy Markdown
Collaborator Author

xuhdev commented Mar 23, 2020

Looks like #25450 is no longer effective regarding VS2017. I would rather restore to the state before #25450 and then find a way to make #35117 (continuation of #33252) compatible with this VS2017 issue. Does it sound good?

@ezyang
Copy link
Copy Markdown
Contributor

ezyang commented Mar 23, 2020

That seems fine, but how did you notice this was no longer working? Master windows seems to be green now.

@xuhdev
Copy link
Copy Markdown
Collaborator Author

xuhdev commented Mar 23, 2020

@ezyang I probably have worded it incorrectly. I meant the "fix" is no longer necessary, because removing the fix still leads to a successful VS2017 build. Not triggering the compiler bug, the fix, which is really a hack, shouldn't be there.

@xuhdev
Copy link
Copy Markdown
Collaborator Author

xuhdev commented Mar 24, 2020

@pytorchbot merge this please

@pytorchbot pytorchbot added the merge-this-please Was marked for merge with @pytorchbot merge this please label Mar 24, 2020
@xuhdev xuhdev force-pushed the ci-all/vs-2017-bug branch from 7c6ca3a to 5dfd405 Compare March 24, 2020 18:38
@xuhdev
Copy link
Copy Markdown
Collaborator Author

xuhdev commented Mar 24, 2020 •
edited
Loading

Originally:

The first commit is the real change, and the second is CI tweaks that should be reverted before merging.

The second commit (CI tweak) has now been reverted.

@xuhdev xuhdev force-pushed the ci-all/vs-2017-bug branch 4 times, most recently from 8178011 to f91bf2a Compare March 27, 2020 19:56
@xuhdev xuhdev mentioned this pull request Mar 28, 2020
Closed
@xuhdev
Copy link
Copy Markdown
Collaborator Author

xuhdev commented Mar 28, 2020 •
edited
Loading

Waiting on #35593

xuhdev added a commit to xuhdev/pytorch that referenced this pull request Mar 29, 2020
@xuhdev
float2::x and float2::y may not be the same as float on ROCm
a4c7075 
This causes ambiguity and can be triggered sometimes (e.g., by pytorch#35217). Explicitly convert them to float.

    error: conditional expression is ambiguous; 'const
    hip_impl::Scalar_accessor<float, Native_vec_, 0>' can be converted to
    'float' and vice versa
facebook-github-bot pushed a commit that referenced this pull request Mar 30, 2020
@xuhdev @facebook-github-bot
float2::x and float2::y may not be the same as float on ROCm (#35593)
788ef93 
Summary:
This causes ambiguity and can be triggered sometimes (e.g., by #35217). Explicitly convert them to float.

    error: conditional expression is ambiguous; 'const
    hip_impl::Scalar_accessor<float, Native_vec_, 0>' can be converted to
    'float' and vice versa
Pull Request resolved: #35593

Differential Revision: D20735663

Pulled By: ezyang

fbshipit-source-id: ae6a38a08e59821bae13eb0b9f9bdf21a008d5c0
@xuhdev xuhdev force-pushed the ci-all/vs-2017-bug branch from f91bf2a to 0993da2 Compare March 30, 2020 21:02
@xuhdev
Copy link
Copy Markdown
Collaborator Author

xuhdev commented Mar 31, 2020

All tests have passed now

gchanan pushed a commit to gchanan/pytorch that referenced this pull request Mar 31, 2020
@xuhdev @gchanan
[v1.5.0] float2::x and float2::y may not be the same as float on ROCm (…
8bd6fad 
…pytorch#35593)

Summary:
This causes ambiguity and can be triggered sometimes (e.g., by pytorch#35217). Explicitly convert them to float.

    error: conditional expression is ambiguous; 'const
    hip_impl::Scalar_accessor<float, Native_vec_, 0>' can be converted to
    'float' and vice versa
Pull Request resolved: pytorch#35593

Differential Revision: D20735663

Pulled By: ezyang

fbshipit-source-id: ae6a38a08e59821bae13eb0b9f9bdf21a008d5c0
@gchanan gchanan mentioned this pull request Mar 31, 2020
Merged
gchanan pushed a commit that referenced this pull request Mar 31, 2020
@xuhdev @gchanan
[v1.5.0] float2::x and float2::y may not be the same as float on ROCm (…
165403f 
…#35593)

Summary:
This causes ambiguity and can be triggered sometimes (e.g., by #35217). Explicitly convert them to float.

    error: conditional expression is ambiguous; 'const
    hip_impl::Scalar_accessor<float, Native_vec_, 0>' can be converted to
    'float' and vice versa
Pull Request resolved: #35593

Differential Revision: D20735663

Pulled By: ezyang

fbshipit-source-id: ae6a38a08e59821bae13eb0b9f9bdf21a008d5c0
@xuhdev xuhdev added the module: windows Windows support for PyTorch label Apr 1, 2020
facebook-github-bot
facebook-github-bot reviewed Apr 1, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@xuhdev xuhdev deleted the ci-all/vs-2017-bug branch April 1, 2020 19:52
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Apr 2, 2020

@ezyang merged this pull request in 15326fb.

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@xuhdev @facebook-github-bot
float2::x and float2::y may not be the same as float on ROCm (pytorch…
ad83170 
…#35593)

Summary:
This causes ambiguity and can be triggered sometimes (e.g., by pytorch#35217). Explicitly convert them to float.

    error: conditional expression is ambiguous; 'const
    hip_impl::Scalar_accessor<float, Native_vec_, 0>' can be converted to
    'float' and vice versa
Pull Request resolved: pytorch#35593

Differential Revision: D20735663

Pulled By: ezyang

fbshipit-source-id: ae6a38a08e59821bae13eb0b9f9bdf21a008d5c0
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@xuhdev @facebook-github-bot
Revert "Attempt to fix windows build" (pytorch#35217)
729545c 
Summary:
This reverts commit 6623bd7.
Pull Request resolved: pytorch#35217

Differential Revision: D20793032

Pulled By: ezyang

fbshipit-source-id: 66132f6007db2932aafcbdb09d89101cb944bab1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamesr66a jamesr66a jamesr66a approved these changes

@ezyang ezyang Awaiting requested review from ezyang

+1 more reviewer

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

merge-this-please Was marked for merge with @pytorchbot merge this please Merged module: windows Windows support for PyTorch open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@xuhdev @ezyang @facebook-github-bot @jamesr66a @pytorchbot @mruberry