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

[ML] Avoiding C++17 warnings for Windows builds #2120

Closed
droberts195 opened this issue Nov 8, 2021 · 7 comments
Closed

[ML] Avoiding C++17 warnings for Windows builds #2120

droberts195 opened this issue Nov 8, 2021 · 7 comments
Assignees
Labels
Milestone

Comments

@droberts195
Copy link
Contributor

droberts195 commented Nov 8, 2021

Our Windows build logs are currently warning of a number of C++ features we are using that are deprecated in C++17 and removed in C++20. We should try to stop using these before we upgrade to C++20 (although if we cannot then it may be possible to continue to use the deprecated features by defining certain macros).

In our own code we have:

C:\Users\jenkins\workspace\elastic+machine-learning+main+windows+debug\include\core/Concurrency.h(139,18): warning C4996: 'std::result_of_t': warning STL4014: std::result_of and std::result_of_t are deprecated in C++17. They are superseded by std::invoke_result and std::invoke_result_t. You can define _SILENCE_CXX17_RESULT_OF_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\Users\jenkins\workspace\elastic+machine-learning+main+windows+debug\include\core/Concurrency.h(141,20): warning C4996: 'std::result_of_t': warning STL4014: std::result_of and std::result_of_t are deprecated in C++17. They are superseded by std::invoke_result and std::invoke_result_t. You can define _SILENCE_CXX17_RESULT_OF_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
CMemoryUsageTest.cc(244,29): warning C4996: 'std::allocator<void>::const_pointer': warning STL4010: Various members of std::allocator are deprecated in C++17. Use std::allocator_traits instead of accessing these members directly. You can define _SILENCE_CXX17_OLD_ALLOCATOR_MEMBERS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.

These should be easy to avoid by changing our code to comply with C++20.

In Boost we have:

C:\usr\local\include\boost-1_77\boost/variant/variant.hpp(1003,5): warning C4996: 'std::_Weak_result_type<std::_Weak_binary_args<ml::maths::common::`anonymous-namespace'::CBinaryVisitor<double,ml::maths::common::`anonymous-namespace'::SPdf>,void>,void>::result_type': warning STL4007: Many result_type typedefs and all argument_type, first_argument_type, and second_argument_type typedefs are deprecated in C++17. You can define _SILENCE_CXX17_ADAPTOR_TYPEDEFS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\usr\local\include\boost-1_77\boost/unordered/detail/implementation.hpp(1452,7): warning C4996: 'std::allocator<std::pair<const std::basic_string<char,std::char_traits<char>,std::allocator<char>>,std::basic_string<char,std::char_traits<char>,std::allocator<char>>>>::is_always_equal': warning STL4010: Various members of std::allocator are deprecated in C++17. Use std::allocator_traits instead of accessing these members directly. You can define _SILENCE_CXX17_OLD_ALLOCATOR_MEMBERS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.

For the boost::variant problem we should be able to switch to std::variant to avoid it. Our files that use boost::variant are:

model/CModelTools.h:            boost::variant<maths::common::CJointProbabilityOfLessLikelySamples, maths::common::CProbabilityOfExtremeSample>;
maths/common/CBjkstUniqueValues.h:    using TUInt32VecOrSketch = boost::variant<TUInt32Vec, SSketch>;
maths/common/CMixtureDistribution.h:        boost::variant<boost::math::normal_distribution<>, boost::math::gamma_distribution<>, boost::math::lognormal_distribution<>>;
maths/common/CSetTools.h:        using TSizeOrSizeSet = boost::variant<std::size_t, TSizeSet>;
maths/time_series/CCountMinSketch.h:    using TUInt32FloatPrVecOrSketch = boost::variant<TUInt32FloatPrVec, SSketch>;

std::variant is not exactly the same as boost::variant, but it's close, so hopefully it won't be too much work to switch.

For the other Boost issue in the unordered detail, there's an issue open for it and somebody said they would fix it on a mailing list. They didn't follow up on their promise, but since it sounds like a 1 or 2 line fix and the mailing list states exactly what to do maybe we can contribute the fix to Boost.

Then there are a couple of issues with Eigen:

C:\Users\jenkins\workspace\elastic+machine-learning+main+windows+debug\3rd_party\eigen\Eigen\src/Core/util/Meta.h(349,2): warning C4996: 'std::equal_to<double>::result_type': warning STL4007: Many result_type typedefs and all argument_type, first_argument_type, and second_argument_type typedefs are deprecated in C++17. You can define _SILENCE_CXX17_ADAPTOR_TYPEDEFS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\Users\jenkins\workspace\elastic+machine-learning+main+windows+debug\3rd_party\eigen\Eigen\src/Core/util/Meta.h(349,2): warning C4996: 'std::equal_to<ml::core::CFloatStorage>::result_type': warning STL4007: Many result_type typedefs and all argument_type, first_argument_type, and second_argument_type typedefs are deprecated in C++17. You can define _SILENCE_CXX17_ADAPTOR_TYPEDEFS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.

Possibly we can upgrade Eigen to avoid these, or, if not, maybe we can contribute a fix there too.

Finally, there are a number of issues in PyTorch:

C:\usr\local\include\pytorch\ATen/core/List.h(106,40): warning C4996: 'std::iterator<std::random_access_iterator_tag,T,ptrdiff_t,T *,T &>': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\usr\local\include\pytorch\ATen/core/List.h(156,3): warning C4996: 'std::iterator<std::random_access_iterator_tag,T,ptrdiff_t,T *,T &>::difference_type': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\usr\local\include\pytorch\c10/util/Optional.h(1129,3): warning C4996: 'std::_Conditionally_enabled_hash<_Kty,true>::result_type': warning STL4007: Many result_type typedefs and all argument_type, first_argument_type, and second_argument_type typedefs are deprecated in C++17. You can define _SILENCE_CXX17_ADAPTOR_TYPEDEFS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\usr\local\include\pytorch\c10/util/TypeList.h(502,26): warning C4996: 'std::result_of_t': warning STL4014: std::result_of and std::result_of_t are deprecated in C++17. They are superseded by std::invoke_result and std::invoke_result_t. You can define _SILENCE_CXX17_RESULT_OF_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\usr\local\include\pytorch\c10/util/irange.h(19,32): warning C4996: 'std::iterator<std::input_iterator_tag,I,ptrdiff_t,I *,I &>': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.

PyTorch itself builds with C++14, so won't see these. We want their headers to also work when we include them in C++20 code. Probably the first step is to see if there's already an issue open against PyTorch for this and open one if not. If it's too controversial for them to adjust their code to be compatible with C++20 then maybe this is where we can use the macros for allowing removed features to still be used, i.e. define _HAS_FEATURES_REMOVED_IN_CXX20, but only when building pytorch_inference.

@droberts195 droberts195 added the :ml label Nov 8, 2021
@edsavage edsavage self-assigned this Nov 8, 2021
edsavage added a commit to edsavage/ml-cpp that referenced this issue Nov 24, 2021
Eigen 3.4.0 has better support for more modern C++ language features and
as such quietens a few C++17 deprecation warnings encountered with
version 3.3.7.

Relates to elastic#2120
edsavage added a commit that referenced this issue Dec 1, 2021
Eigen 3.4.0 has better support for more modern C++ language features and
as such quietens a few C++17 deprecation warnings encountered with
version 3.3.7.

Note that Eigen 3.4.0 handles NaN values more robustly than in previous versions.
Ensure to check the result of JacobiSVD::compute before proceeding to
avoid working with uninitialised values.

Relates to #2120
@edsavage
Copy link
Contributor

edsavage commented Dec 2, 2021

The deprecation warning from Boost unordered detail will be fixed in an upcoming major reworking of the unordered library implementation. See comment on this PR - boostorg/unordered#52

@edsavage
Copy link
Contributor

edsavage commented Dec 2, 2021

An issue has been opened on the PyTorch repo detailing the deprecation warnings we are seeing - pytorch/pytorch#69290.

@edsavage edsavage changed the title [ML] Avoiding C++17 deprecation warnings [ML] Avoiding C++17 warnings Dec 3, 2021
@edsavage edsavage changed the title [ML] Avoiding C++17 warnings [ML] Avoiding C++17 warnings for Windows builds Dec 3, 2021
edsavage added a commit to edsavage/ml-cpp that referenced this issue Dec 6, 2021
On Windows a number of deprecation warnings are seen emanating from
PyTorch header files (See elastic#2120). An issue has been opened on the
PyTorch repo requesting that these deprecations be addressed
(pytorch/pytorch#69290). While the issue has
been triaged it is suspected that it will be some time before fixes are
forthcoming. These changes silence such deprecation warnings for
pytorch_inference only.
edsavage added a commit that referenced this issue Dec 8, 2021
On Windows a number of deprecation warnings are seen emanating from
PyTorch header files (See #2120). An issue has been opened on the
PyTorch repo requesting that these deprecations be addressed
(pytorch/pytorch#69290). While the issue has
been triaged it is suspected that it will be some time before fixes are
forthcoming. These changes silence such deprecation warnings for
pytorch_inference only.
@droberts195
Copy link
Contributor Author

droberts195 commented Oct 18, 2023

PyTorch itself builds with C++14, so won't see these.

In PyTorch 2.1 they're using C++17 - see pytorch/pytorch#100557 - so the PyTorch side of this should no longer be a problem after #2587.

@droberts195
Copy link
Contributor Author

After #2612 is merged we should check if #2152 can be reverted. If it can then maybe this issue can be closed now?

@sophiec20
Copy link

@edsavage can we close this one yet?

@edsavage
Copy link
Contributor

edsavage commented Apr 10, 2024

can we close this one yet?

Yes, this one can now be closed.

However, I do notice a number of new warnings in the Windows build logs, mostly related to Boost::Json. I will open one or more separate issues to cover those.

@sophiec20 sophiec20 added this to the 8.14 milestone Apr 24, 2024
@edsavage
Copy link
Contributor

However, I do notice a number of new warnings in the Windows build logs, mostly related to Boost::Json. I will open one or more separate issues to cover those.

These remaining warnings have been addressed by #2653, hence this issue can now be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants