Changes to build the examples under MSVC - #1
mcleary wants to merge 4 commits into
Conversation
Sorry, something went wrong.
Rbiessy
left a comment
There was a problem hiding this comment.
For the addition to the eigen.patch I think we should push them in Eigen instead. I'd like to keep a patch file as small as possible.
Do you want to submit them yourself on Eigen? The need of std::remove_reference is definitely something to investigate with Mehdi. I can take care of this later if you can't spend time on this.
Sorry, something went wrong.
| endforeach() | ||
|
|
||
| # Need to define _USE_MATH_DEFINES for M_PI when building on MSVC | ||
| if(MSVC) |
There was a problem hiding this comment.
I decided to remove the usage of M_PI so you can remove that.
Sorry, something went wrong.
| foreach(SOURCE ${SOURCES}) | ||
| process_source_name(${SOURCE}) | ||
| add_executable(${TARGET_NAME} ${SOURCE}) | ||
| add_executable(${TARGET_NAME} ${SOURCE}) |
There was a problem hiding this comment.
Can you remove the spaces here?
Sorry, something went wrong.
|
|
||
| if (max_nb_iter == 0) | ||
| max_nb_iter = std::max(10000000LU, m > INT_MAX/100 ? INT_MAX : 100*m); | ||
| max_nb_iter = std::max(10000000ULL, m > std::numeric_limits<size_t>::max()/100ULL ? std::numeric_limits<size_t>::max() : 100ULL*m); |
There was a problem hiding this comment.
This will break on Unix since the index type is unsigned long there. I'd like to know why it is long long on Windows, maybe Mehdi knows more about this.
If possible I'd like to have a clean solution on the Eigen side for this, if it's not possible I'll use decltype instead here.
Sorry, something went wrong.
There was a problem hiding this comment.
size_t is a long long on Windows. You could do a static cast into the size_t type but that seems ugly. You should probably just make sure that all the types in that call are the same, whatever they might be. I guess m might be the problem here?
Sorry, something went wrong.
There was a problem hiding this comment.
Oh ok, that's annoying. m is part of the problem yes, I have 2 constants that should be of type size_t (for min and max type deduction). It's not great that I have these constants but that's an issue for another time, I'll have to static_cast them for now.
Thanks!
Sorry, something went wrong.
|
My mistake on the eigen.patch. I will restore the patch to the original version. |
Sorry, something went wrong.
|
It's alright. At least people can see what needs to be done if they want to try to make it work as well :) |
Sorry, something went wrong.
|
@Rbiessy is this patch still relevant? If it is let me know and I will update it. |
Sorry, something went wrong.
|
Well I would definitely want to merge it if I had time to work on this project but I don't think it will happen. |
Sorry, something went wrong.
The FindComputeCpp.cmake was updated to the latest version.
The download mechanism for the mnist database was changed to use CMake file command.
The file smo.hpp had some incompatibilities with MSVC.