nimble-dev / nimble

The base NIMBLE package for R
http://R-nimble.org
BSD 3-Clause "New" or "Revised" License
157 stars 24 forks source link

installation issue related to clang11 #1001

Closed paciorek closed 4 years ago

paciorek commented 4 years ago

CRAN is now reporting an issue related to clang11 that we'll have to deal with for next release.

https://cran.r-project.org/web/checks/check_results_nimble.html

perrydv commented 4 years ago

On first glance these look like they arise from Eigen code. But maybe there is a flag or setting we can use to manage them.

paciorek commented 4 years ago

The "additional issues" link is simply titled "clang11" and it leads the to log file attached here. nimble.log

perrydv commented 4 years ago

The "additional issues" itself is a link, which then has a link for the clang11 check which has a URL which lists some configuration information. Are you able to go from this information to know how to reproduce the errors? The internet seems to say that clang11 is not yet released.

It could be that simply updating Eigen, or switching to pick it up from RcppEigen, as we do in nCompiler, would somehow resolve this. But if we can't reproduce the error, it will be hard to resolve it.

paciorek commented 4 years ago

I hope/suspect we could replicate via rhub:

 rhub::check("mypackage_1.0.0.tar.gz", platform = "fedora-clang-devel")

Of course that won't get us an environment in which we can work manually. If necessary, I could probably get a facsimile of Ripley's environment going in a Docker container. Seeing the output from rhub might also help in terms of seeing how to configure an environment in which to reproduce.

I didn't look fully, but I see multiple packages (e.g, boom, picasso) that make use of Eigen (including at least one via RcppEigen, namely prophet) that all have this issue. However the package fastglm that uses RcppEigen does not have this issue.

So we should probably search around more and try to dive into how different packages are/are not facing this and any chatter about dealing with this.

perrydv commented 4 years ago

Oh, that's great. I'll try that.

Eigen is all-header (templates), and I think RcppEigen is primarily wrappers to Eigen types, so it is possible that building RcppEigen package does not actually compile much Eigen code and might not trigger it for that reason.

paciorek commented 4 years ago

Just to make sure I was clear: prophet has the issue even though it uses RcppEigen. I didn't look comprehensively across all packages that depend on RcppEigen.

perrydv commented 4 years ago

I had a hard time getting rhub to work. I did the validate_email step and then the rhub call you suggested, but with my email. It spun for a long time with no update. The linked page (generated from rhub call) showed nothing, just "..." for status, and then timed out repeatedly. Are you able to get a result back from rhub? It's not clear to me from platforms() that one of these is the one generating the clang11 error.

paciorek commented 4 years ago

I thought we'd want this: fedora-clang-devel, based on this: https://www.stats.ox.ac.uk/pub/bdr/Rconfig/r-devel-linux-x86_64-fedora-clang

but I think you probably thought more about it than me.

On Thu, May 7, 2020 at 12:53 PM perrydv notifications@github.com wrote:

I had a hard time getting rhub to work. I did the validate_email step and then the rhub call you suggested, but with my email. It spun for a long time with no update. The linked page (generated from rhub call) showed nothing, just "..." for status, and then timed out repeatedly. Are you able to get a result back from rhub? It's not clear to me from platforms() that one of these is the one generating the clang11 error.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nimble-dev/nimble/issues/1001#issuecomment-625463301, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATCXXRBHEUEDEUN6HUUH5LRQMGUJANCNFSM4MXLRZ5A .

perrydv commented 4 years ago

We should probably talk. I don't know what to do with your last comment.

paciorek commented 4 years ago

You confused me too. :)

I just meant that the rhub test you ran (see below from your other email) seemed like what we wanted, in that running on Fedora, with clang, using R-devel seems like it mimics the settings indicated here: https://www.stats.ox.ac.uk/pub/bdr/Rconfig/r-devel-linux-x86_64-fedora-clang (which is apparently the settings under which CRAN's clang test occurs).

Build ID: nimble_0.10.0.tar.gz-f3e5774888de49c8869868de4b6fd7a2 Platform: Fedora Linux, R-devel, clang, gfortran Submitted: 1 hour 18 minutes 17 seconds ago Build time: 40 minutes 53 seconds

On Thu, May 7, 2020 at 3:17 PM perrydv notifications@github.com wrote:

We should probably talk. I don't know what to do with your last comment.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nimble-dev/nimble/issues/1001#issuecomment-625525138, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATCXXT7IFYBTBQTI6TV6O3RQMXQXANCNFSM4MXLRZ5A .

perrydv commented 4 years ago

We could see it how it goes with CRAN ignoring this issue for now. As far as I can see, clang11 is not released: https://en.wikipedia.org/wiki/Clang#Status_history

paciorek commented 4 years ago

The clang11 test on Ripley's machine seems to use "clang-trunk" with clang10, which seems to have features to be included in clang11. rhub's fedora-clang-devel platform does not seem to use clang-trunk, which is presumably why nimble passes on rhub.

I've emailed the maintainer of prophet, which was updated 2020-04-29 to see if the clang11 issue was active for them at the time they sent that update to CRAN.

paciorek commented 4 years ago

In early May multinet had the clang11 issue but did not in the most recent (3.3) release on May 8. See below for a diff of commits on https://bitbucket.org/uuinfolab/r_multinet that seems to show what was done.

@perrydv if it seems like adding #include sstream seems plausible, we might try to do that without the can of worms of recreating Ripley's setup and submit to CRAN and see what happens. I would then respond to CRAN's message and have some language about thinking we've addressed the problem.

diff --git a/src/eigen3/Eigen/src/Core/IO.h b/src/eigen3/Eigen/src/Core/IO.h
index 4b0b01e..f598e69 100644
--- a/src/eigen3/Eigen/src/Core/IO.h
+++ b/src/eigen3/Eigen/src/Core/IO.h
@@ -11,6 +11,8 @@
 #ifndef EIGEN_IO_H
 #define EIGEN_IO_H

+#include <sstream>
+
 namespace Eigen {

 enum { DontAlignCols = 1 };
diff --git a/src/eigen3/Eigen/src/SparseLU/SparseLU.h b/src/eigen3/Eigen/src/SparseLU/SparseLU.h
index 72cc84c..42713eb 100644
--- a/src/eigen3/Eigen/src/SparseLU/SparseLU.h
+++ b/src/eigen3/Eigen/src/SparseLU/SparseLU.h
@@ -12,6 +12,8 @@
 #ifndef EIGEN_SPARSE_LU_H
 #define EIGEN_SPARSE_LU_H

+#include <sstream>
+
paciorek commented 4 years ago

We submitted 0.9.1 and CRAN accepted it after asking whether we had fixed this issue - I responded 'no', but said we were working on it and asked that we be able to delay until next release.

CRAN then said the issue seemed to have been fixed and I no longer see the issue on our package results page. Not sure what happened, but closing this for now.