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

scikit does not compile with Visual Studio 16 2019 #142

Closed
galleon opened this issue Dec 8, 2021 · 3 comments · Fixed by #173
Closed

scikit does not compile with Visual Studio 16 2019 #142

galleon opened this issue Dec 8, 2021 · 3 comments · Fixed by #173
Assignees

Comments

@galleon
Copy link
Contributor

galleon commented Dec 8, 2021

🐛 Bug

When.moving to windows-latest on GitHubActions we got the following environment:
-- Building for: Visual Studio 16 2019
-- The C compiler identification is MSVC 19.29.30137.0
-- The CXX compiler identification is MSVC 19.29.30137.0

This causes the following error:

D:/a/scikit-decide/scikit-decide/cpp/src/hub/solver/riw/impl/riw_impl.hh(585,5): message : existing declarations [D:\a\scikit-decide\scikit-decide\build\temp.win-amd64-3.7\Release_skdecide\hub\__skdecide_hub_cpp\src\hub\solver\riw\py_riw.vcxproj]
D:/a/scikit-decide/scikit-decide/cpp/src/hub/solver/riw/impl/riw_impl.hh(585,5): message : 'skdecide::RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::WidthSolver::WidthSolver(skdecide::RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy> &,Tdomain &,const std::function<std::unique_ptr<Tfeature_vector,std::default_delete<Tfeature_vector>>(Tdomain &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::Domain::State &,const size_t *)> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<size_t> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<size_t> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<size_t> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<double> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<size_t> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<double> &,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<double> &,std::list<double,std::allocator<double>> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<double> &,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<double> &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<size_t> &,SetTypeDeducer<skdecide::RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::Node,Thashing_policy<Tdomain,Tfeature_vector>>::Set &,Trollout_policy<Tdomain> &,Texecution_policy &,std::mt19937 &,WidthSolver::ExecutionPolicy::Mutex &,WidthSolver::ExecutionPolicy::Mutex &,WidthSolver::ExecutionPolicy::Mutex &,const RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::ExecutionPolicy::atomic<bool> &,const skdecide::RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::WatchdogFunctor &)' [D:\a\scikit-decide\scikit-decide\build\temp.win-amd64-3.7\Release_skdecide\hub\__skdecide_hub_cpp\src\hub\solver\riw\py_riw.vcxproj]
D:/a/scikit-decide/scikit-decide/cpp/src/hub/solver/riw/impl/riw_impl.hh(603,62): error C2244: 'skdecide::RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::WidthSolver::solve': unable to match function definition to an existing declaration [D:\a\scikit-decide\scikit-decide\build\temp.win-amd64-3.7\Release_skdecide\hub\__skdecide_hub_cpp\src\hub\solver\riw\py_riw.vcxproj]
D:/a/scikit-decide/scikit-decide/cpp/src/hub/solver/riw/impl/riw_impl.hh(599): message : see declaration of 'skdecide::RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::WidthSolver::solve' [D:\a\scikit-decide\scikit-decide\build\temp.win-amd64-3.7\Release_skdecide\hub\__skdecide_hub_cpp\src\hub\solver\riw\py_riw.vcxproj]
D:/a/scikit-decide/scikit-decide/cpp/src/hub/solver/riw/impl/riw_impl.hh(603,62): message : definition [D:\a\scikit-decide\scikit-decide\build\temp.win-amd64-3.7\Release_skdecide\hub\__skdecide_hub_cpp\src\hub\solver\riw\py_riw.vcxproj]
D:/a/scikit-decide/scikit-decide/cpp/src/hub/solver/riw/impl/riw_impl.hh(603,62): message : 'bool skdecide::RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::WidthSolver::solve(const WidthSolver::Domain::State &,const std::chrono::time_point<std::chrono::steady_clock,std::chrono::nanoseconds> &,WidthSolver::ExecutionPolicy::atomic<size_t> &,std::vector<std::unordered_map<std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>,size_t,boost::hash<std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>>,std::equal_to<std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>>,std::allocator<std::pair<const std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>,size_t>>>,std::allocator<std::unordered_map<std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>,size_t,boost::hash<std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>>,std::equal_to<std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>>,std::allocator<std::pair<const std::vector<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>,std::allocator<std::pair<size_t,RIWSolver<Tdomain,Tfeature_vector,Thashing_policy,Trollout_policy,Texecution_policy>::FeatureVector::value_type>>>,size_t>>>>> &)' [D:\a\scikit-decide\scikit-decide\build\temp.win-amd64-3.7\Release_skdecide\hub\__skdecide_hub_cpp\src\hub\solver\riw\py_riw.vcxproj]
D:/a/scikit-decide/scikit-decide/cpp/src/hub/solver/riw/impl/riw_impl.hh(603,62): message : existing declarations [D:\a\scikit-decide\scikit-decide\build\temp.win-amd64-3.7\Release_skdecide\hub\__skdecide_hub_cpp\src\hub\solver\riw\py_riw.vcxproj]
@fteicht fteicht changed the title scikit does compile with Visual Studio 16 2019 scikit does not compile with Visual Studio 16 2019 Dec 8, 2021
@fteicht
Copy link
Collaborator

fteicht commented Jan 7, 2022

Thanks for the report.

The compilation error regarding RIW is easy to fix: it is due to the fact that MSVC 19.29 does not allow function members defined inside the class to use typedef members defined after the function members.

So we just need to move lines from that one to that one in riw.hh before the first function member of the class.

However, there remains an issue with the compilation of MCTS due to MSVC 19.28+ requiring the compilation flag /Zc:lambda to compile default lambdas used in template class constructors and referencing the class' template types, as in this line of mcts.hh. Other compilers don't need to activate such feature flags to compile this piece of code. What could be considered as a bug has been already filed to MSVC's development team: see this bug report which is exactly the same issue as the one preventing from compiling the aforementioned line in mcts.hh.

We need here to decide if we want to explicitly pass the /Zc:lambda compilation flag to MSVC when we compile on Windows, knowing that this flag exists since MSVC 16.8 as mentioned here. Would scikit-decide also compile with earlier version of MSVC, or can we assume that we anyway need template meta-programming features that would be only available for versions of MSVC higher than 16.8?

@fteicht
Copy link
Collaborator

fteicht commented Jan 7, 2022

So the RIW compilation error is a no-brainer, whereas the MCTS one requires to make assumptions on the version of MSVC we are using: we need to reject MSVC versions lower than 16.8 and to add the /Zc:lambda compilation flag when compiling with MSVC.
I will shortly propose a pull request which does exactly that.

@dbarbier
Copy link
Contributor

dbarbier commented Jan 10, 2022

@fteicht Please push your fix here or on your repo, I will make it work for any MSVC version.

@fteicht fteicht mentioned this issue Jan 11, 2022
@fteicht fteicht linked a pull request Jan 11, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants