-
-
Notifications
You must be signed in to change notification settings - Fork 208
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
Issue with Rcpp and Intel compiler exposed by building haven #640
Comments
You are evidently on 3.2.3 as we can tell. That may mean that you are also on an older version of Rcpp. Here, you didn't even tell which version it was. Could you do, and if you have not done that yet, also try Rcpp 0.12.9 which is now current? |
Thanks for the quick response. Yes, I am on 3.2.3 and I have Rcpp 0.12.9 installed. I even tried with the current github version, but that one does not work either. Please let me know if you need any further information. |
Well ... I can't help you as I don't have your compiler. From the lack of the error message, it doesn't like the |
The problem is that See e.g. https://software.intel.com/en-us/articles/cdiag858 We should define something like I'm guessing that |
A quick recursive grep shows that R> Rcpp::evalCpp("__cplusplus")
[1] 199711
R> and while I was trying to redo that as C++11 @kevinushey already gave you a better answer. Edit: For reference:
|
Actually we just need to define RCPP_CONSTEXPR as empty for non c++11 builds. No need to have const in front of the return type I think. |
Do we use E.g. it's legal to write things like:
We should probably make sure that, if we do, we replace those usages of |
So @wds15: as a local test, please edit that file to read #if __cplusplus >= 201103L
# define RCPP_CONSTEXPR constexpr
#else
# define RCPP_CONSTEXPR
#endif @kevinushey @dcdillon We could possibly move the declaration to one of the compiler or preprocessor directories. |
Thanks so much for the effort. I tried as you suggested, but it did not work. I then did a few tests in small tests as follows: compilation with vanilla Rcpp/alogrithm.hpp and turned C++11 on:
So it looks to me as if the C++11 detection which you do does not work for Intel. OK, so I thought lets force the constexpr definition (by just defining as constexpr without the ifdef) and run with C++11 again:
Now the constexpr is used, but there is still an error. To complete things I furthermore compiled with no c++11 and forced definition with const. Then I get
Finally, I compile again without C++11, but forcing as you suggest RCPP_CONSTEXPR to be empty:
So still not working. I hope this helps to pin down the issue here. Let me know if you need further tests. BTW, the boost config library is quite reliable to detect these compiler quirks. |
We cannot / do not want to have a build dependency on Boost. Above, you had one case where you died in DFreader. That is a haven file. Looks like you made progress there. I encouragement to work that angle as nobody here uses the Intel compiler. Or if you must use Rcpp, try some older releases, maybe as far back as 0.12.5. |
Sure, I understand the desire to minimize dependencies. The C++11 detection you do does not work for Intel. You may want to include
to make Intel pickup the C++11 definition if C++11 is switched on. I know that Intel compilers can be a problem and my apologies for the inconvenience. It would be great to include the above into Rcpp. The Intel compiler just happens to be the one on our production system. If I were to pick the compiler in production it would be the one from Rtools (gcc 4.9.3), but changeing that is beyond my control. Thanks for your support. |
Yes, we do have similar tests in other place for this. The file that is giving you trouble is newer, and decided to redo its tests for compiler / C++ standard locally, and we now see a shortcoming. It happens. But we rely on you to find test that works. Are you saying that the above is sufficient now? Also, some casual reading on SO eg here suggest that this is an Intel compiler issue. The standard apparently talks about this in clear terms:
|
The test I outlined above does work here for me, yes. This is not the first time we had issues with Intel being the bad guy here.. this is another occasion to ask to switch to gcc. Still, other Intel users will appreciate if the above could be included. |
Ok. We may still switch to using 'empty' instead of There used to be a (real or perceived) performance gap between |
I think defining it empty cause errors inside of Rcpp as I tested above. My preferred choice for a production system would be gcc 4.9.3 (like Rtools) combined with the Intel MKL BLAS/LAPACK libraries. That should give almost all performance gain there is to get from this Intel stuff, but still maintain open-source R compatibility. |
Granted that just adding the 'or' is simpler / less of a change. And yes, the MKL are good. So much so that Microsoft is twisting the language in advertising their Microsoft R that it "needs to be recompiled for MKL". Nope. BLAS interface. They still add MKL plus some out of memory extensions, but the MKL can be had without them too. |
Hmm.. Hadley has no clue on this. Anyway. Thanks much. |
When installing haven 1.0.0 with the Intel compiler I endup getting this error in Rcpp files. Hadley suggested this is an Rcpp issue (I reported this for haven already):
Installing haven 1.0.0 with the Intel compiler (icpc (ICC) 15.0.3 20150407) I get this error log:
Here is my session info:
Could it be that some C++ being used is not fully standard?
The text was updated successfully, but these errors were encountered: