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

Fix compiling with C++17 #343

Closed
wants to merge 1 commit into from

Conversation

mantaionut
Copy link
Contributor

std::random_shuffle is deprecated in C++14 and removed in C++17.
This commit replaces it by std::shuffle.

std::random_shuffle was removed in C++17. This commit replaces it by
std::shuffle.
@facebook-github-bot
Copy link

@malfet has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mantaionut
Copy link
Contributor Author

Hi, I see that a test failed but I don't have access to the details. Is there anything I can do to help move this PR forward?

pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Dec 8, 2022
With CUDA-10.2 gone we can finally do it!

This PR mostly contains build system related changes, invasive functional ones are to be followed.
Among many expected tweaks to the build system, here are few unexpected ones:
 - Force onnx_proto project to be updated to C++17 to avoid `duplicate symbols` error when compiled by gcc-7.5.0, as storage rule for `constexpr` changed in C++17, but gcc does not seem to follow it
 - Do not use `std::apply` on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when `std::apply` is invoked from CUDA code.
 - `std::decay_t` -> `::std::decay_t` and `std::move`->`::std::move` as VC++ for some reason claims that `std` symbol is ambigious
 - Disable use of `std::aligned_alloc` on Android, as its `libc++` does not implement it.

Some prerequisites:
 - #89297
 - #89605
 - #90228
 - #90389
 - #90379
 - #89570
 - facebookincubator/gloo#336
 - facebookincubator/gloo#343
 - pytorch/builder@919676f

Fixes #56055

Pull Request resolved: #85969
Approved by: https://github.com/ezyang, https://github.com/kulinseth
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
With CUDA-10.2 gone we can finally do it!

This PR mostly contains build system related changes, invasive functional ones are to be followed.
Among many expected tweaks to the build system, here are few unexpected ones:
 - Force onnx_proto project to be updated to C++17 to avoid `duplicate symbols` error when compiled by gcc-7.5.0, as storage rule for `constexpr` changed in C++17, but gcc does not seem to follow it
 - Do not use `std::apply` on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when `std::apply` is invoked from CUDA code.
 - `std::decay_t` -> `::std::decay_t` and `std::move`->`::std::move` as VC++ for some reason claims that `std` symbol is ambigious
 - Disable use of `std::aligned_alloc` on Android, as its `libc++` does not implement it.

Some prerequisites:
 - pytorch#89297
 - pytorch#89605
 - pytorch#90228
 - pytorch#90389
 - pytorch#90379
 - pytorch#89570
 - facebookincubator/gloo#336
 - facebookincubator/gloo#343
 - pytorch/builder@919676f

Fixes pytorch#56055

Pull Request resolved: pytorch#85969
Approved by: https://github.com/ezyang, https://github.com/kulinseth
pruthvistony pushed a commit to ROCm/pytorch that referenced this pull request Dec 20, 2022
With CUDA-10.2 gone we can finally do it!

This PR mostly contains build system related changes, invasive functional ones are to be followed.
Among many expected tweaks to the build system, here are few unexpected ones:
 - Force onnx_proto project to be updated to C++17 to avoid `duplicate symbols` error when compiled by gcc-7.5.0, as storage rule for `constexpr` changed in C++17, but gcc does not seem to follow it
 - Do not use `std::apply` on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when `std::apply` is invoked from CUDA code.
 - `std::decay_t` -> `::std::decay_t` and `std::move`->`::std::move` as VC++ for some reason claims that `std` symbol is ambigious
 - Disable use of `std::aligned_alloc` on Android, as its `libc++` does not implement it.

Some prerequisites:
 - pytorch#89297
 - pytorch#89605
 - pytorch#90228
 - pytorch#90389
 - pytorch#90379
 - pytorch#89570
 - facebookincubator/gloo#336
 - facebookincubator/gloo#343
 - pytorch/builder@919676f

Fixes pytorch#56055

Pull Request resolved: pytorch#85969
Approved by: https://github.com/ezyang, https://github.com/kulinseth
pruthvistony pushed a commit to ROCm/pytorch that referenced this pull request Jan 3, 2023
With CUDA-10.2 gone we can finally do it!

This PR mostly contains build system related changes, invasive functional ones are to be followed.
Among many expected tweaks to the build system, here are few unexpected ones:
 - Force onnx_proto project to be updated to C++17 to avoid `duplicate symbols` error when compiled by gcc-7.5.0, as storage rule for `constexpr` changed in C++17, but gcc does not seem to follow it
 - Do not use `std::apply` on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when `std::apply` is invoked from CUDA code.
 - `std::decay_t` -> `::std::decay_t` and `std::move`->`::std::move` as VC++ for some reason claims that `std` symbol is ambigious
 - Disable use of `std::aligned_alloc` on Android, as its `libc++` does not implement it.

Some prerequisites:
 - pytorch#89297
 - pytorch#89605
 - pytorch#90228
 - pytorch#90389
 - pytorch#90379
 - pytorch#89570
 - facebookincubator/gloo#336
 - facebookincubator/gloo#343
 - pytorch/builder@919676f

Fixes pytorch#56055

Pull Request resolved: pytorch#85969
Approved by: https://github.com/ezyang, https://github.com/kulinseth
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.

None yet

3 participants