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

R versions to support and which to cover in test suite #161

Open
clhunsen opened this issue Mar 26, 2019 · 7 comments
Open

R versions to support and which to cover in test suite #161

clhunsen opened this issue Mar 26, 2019 · 7 comments

Comments

@clhunsen
Copy link
Collaborator

Description

Originating in #152, the question arose which R versions we should support and which to cover in the test suite.

Motivation

Currently, we explicitly support R version 3.3.1 (see README.md). However, new versions of R are released over time and we should ensure that these work with our network library in principle.

Ideas for the Implementation

There are two work items for this issue (see also the discussion in #152):

  1. We definitely need to cover version 3.3.x in the test suite (i.e., TravisCI), since this version is installed in all our containers!
  2. We need to find a way to reliably and correctly install all dependencies (i.e., R packages) for all R version covered in our .travis.yml. Some dependencies are not available for older R version anymore. See TravisCI tests are failing for R 3.3 #152 (comment) for some further details.

What are your thoughts? Is this an issue that can be stalled until we transform this project itself into a proper R package?

@clhunsen clhunsen added this to the Future milestone Mar 26, 2019
clhunsen added a commit to clhunsen/coronet that referenced this issue Jul 17, 2019
This is related to issue se-sic#161.

Signed-off-by: Claus Hunsen <[email protected]>
clhunsen added a commit to clhunsen/coronet that referenced this issue Jan 24, 2020
This is related to issue se-sic#161.

Signed-off-by: Claus Hunsen <[email protected]>
@clhunsen clhunsen mentioned this issue Jan 24, 2020
6 tasks
@bockthom bockthom mentioned this issue Feb 21, 2020
@bockthom
Copy link
Collaborator

bockthom commented Nov 1, 2020

Currently (for about two weeks), our builds on travis ci fail for R versions 3.3, 3.4, and 3.5. Only 3.6 builds correctly. I had a look at the build logs and identified the following:
(1) For R versions 3.3, 3.4, and 3.5, the build is terminated as the maximum log size is exceeded.
(2) Why is the log size exceeded? It's mostly due to installing lots of R packages and errors during installing them. It looks to me that also R packages are installed that are never used (also not as a dependency, to the best of my knowledge), which fail due to missing or wrong dependencies. On the other hand, some needed R packages cannot be installed any more as they are not available for outdated R versions any more.

As all builds on all branches fail due to the above described problems, I tried to figure out whether there is an easy option on how to fix the problems. To reduce the log size, we could set log level to INFO instead of DEBUG. However, this won't help us in general when the verbose log of installing packages is the reason for exceeding the maximum log length. So, the more important problem is dealing with installing the packages. Unfortunately, I did not find acceptable solutions. It still seems weird to me that two weeks ago everything worked for all R versions 3.3, 3.4, 3.5 and now it does not work for neither of them.

I also had a look at the solutions provided in #152. Most promising approach was the oldr::install.compatible.packages function. However, even installing oldr is not possible for outdated R versions any more - only option would be to pin oldr to a specific version, because then we won't get new versions of this package even for recent R versions. But this also introduces new dependencies which are not necessary, which I really dislike.

As I would like to see green check-marks next to our build logs again, the only practicable solution for now would be to restrict test runs to R version 3.6 (as 3.6.3 is the R version our current container setups use). Later we should also support R version 4, but there may be some braking changes, so I won't give it a try yet. However, even when removing R 3.3, 3.4, 3.5 from the test runs, we might result in a similar problem for R version 3.6 soon. Hence, this will only solve the problem for the short term. Automatically installing a compatible version would be great, why can't R do that automatically?

@clhunsen What is your opinion on that?

@clhunsen
Copy link
Collaborator Author

clhunsen commented Nov 4, 2020

My personal Travis builds

Interestingly, the weekly Travis build for my fork ran successfully just yesterday. 🤔 Maybe, the extra output of the new dependencies (introduced in PR #173) is too much. Maybe, the reason lies somewhere else.

Thank you very much for your insights and thoughts on this, @bockthom. My thoughts and findings follow.

Reducing the log length

I thought about the problem in the last days too and found a possible workaround that may help to reduce the log length: we should install packages with reduced verbosity. The output of the actual tests only makes up ~10k lines, while the package installation makes up over 30k lines, stressing the maximum log length.

I tested some approaches and found that adapting the call to install.packages(...) in install.R directly is the best approach to (temporarily) reduce the log length.

install.packages(..., quiet = TRUE, verbose = TRUE)

The output prints the progress, but does not log any compilation output to the console. Maybe, this helps in the search for problems with the Travis builds.

Output looks like this (for a failing installation):

> install.packages("udunits2", quiet = TRUE, verbose = TRUE)
system (cmd0): /usr/lib64/R/bin/R CMD INSTALL
foundpkgs: udunits2, /tmp/RtmpihNqb0/downloaded_packages/udunits2_0.13.tar.gz
files: /tmp/RtmpihNqb0/downloaded_packages/udunits2_0.13.tar.gz
Warning message:
In install.packages("udunits2", quiet = TRUE, verbose = TRUE) :
  installation of package ‘udunits2’ had non-zero exit status

R 4.0

I have R 4.0 installed on my notebook, the installation of packages passed without any problems, and the test suite of coronet passed completely. So, support for R 4.0 should be no problem, I guess. 🙂

Docker

Maybe, the creation of a Docker container for using coronet in a safe and supported environment is a long-term solution for both adopters/users and Travis tests. 🤔 This would also lay a good basis for controlled updates of coronet, R version, dependencies, system, ... Just an idea. 🤷‍♂️

@bockthom
Copy link
Collaborator

bockthom commented Nov 4, 2020

Thanks for your answer @clhunsen. I have a few comments on it:

My personal Travis builds

Interestingly, the weekly Travis build for my fork ran successfully just yesterday. thinking Maybe, the extra output of the new dependencies (introduced in PR #173) is too much.

Nope. This is not related to PR #173. The travis build fails also for the current dev and master branches which are unchanged since February 2020, and which had successful builds until two weeks ago. (And even #173 had successful builds until two weeks ago, and there has essentially nothing changed in this PR since then.) Hence, the new dependencies introduced in #173 cannot have caused the problem.

The difference between the builds on your fork and the builds on the main project is: You still use cached package installations, the main project has to rebuild the package installations. I don't know what has triggered that, but rebuilding the packages is not possible any more and results in lots of errors bloating the log. I'll promise that your fork will also result in build failures if the packages would have to been rebuilt. You're just in luck that you still can rely on travis's cached package installations.

And reinstalling the packages also installs lots of packages that are never used, I don't know why. Most of these packages result in compile errors. So, eventually, I am not sure if just reducing the log length will suffice to get green check-marks in the end, as those package installations may result in weird behavior...

Reducing the log length

I thought about the problem in the last days too and found a possible workaround that may help to reduce the log length: we should install packages with reduced verbosity. The output of the actual tests only makes up ~10k lines, while the package installation makes up over 30k lines, stressing the maximum log length.

True. But most of those over 30k lines are a result of the failing package installations. Normal, succeeding package installations result in a smaller log output (but may still make up more lines than the actual tests).

I tested some approaches and found that adapting the call to install.packages(...) in install.R directly is the best approach to (temporarily) reduce the log length.

install.packages(..., quiet = TRUE, verbose = TRUE)

Sounds good! Thanks for thinking about that and testing this solution. Independent from the current issue, this will help us for the future when adding new tests may bring us closer to the maximum log length in general. So, it's definitely worth trying to be well positioned for future enhancements.

R 4.0

I have R 4.0 installed on my notebook, the installation of packages passed without any problems, and the test suite of coronet passed completely. So, support for R 4.0 should be no problem, I guess. slightly_smiling_face

Short answer: Sounds good! Thanks for testing that. So, whenever touching the travis configuration next time, I will add R 4.0 to the R versions used for the builds and tests.

Docker

Maybe, the creation of a Docker container for using coronet in a safe and supported environment is a long-term solution for both adopters/users and Travis tests. thinking This would also lay a good basis for controlled updates of coronet, R version, dependencies, system, ... Just an idea. man_shrugging

Good idea. But it is costly in terms of preparation time (which we don't have at the moment), and which does not help us with the issues we are facing at the moment, i.e., figuring out why package installations fail (or, to be precise why the packages have to be rebuilt and there are lots of package installations failing now which never have been installed or failed before...) But I will keep the Docker idea in mind for future changes.

bockthom added a commit to bockthom/coronet that referenced this issue Nov 8, 2020
Travis builds currently fail (for a few weeks) due to two different reasons:

On the one hand, the log output exceeds the maximum log length of travis,
resulting in failing builds. This is prevented by setting the parameters
`verbose` and `quiet` to `TRUE`, as then the package compilation log is
not printed to the log but the result of package installation is clearly
perceptible in the log.

On the other hand, travis installs lots of packages which are not necessary
as they are just suggestions of other packages but not necessary dependencies.
Installing such unnecessary packages my lead to problems. This is
circumvented by setting the `dependencies` parameter to `NA` as this results
in installing only necessary dependencies and imports, whereas the former
`TRUE` value of this parameter also led to installing suggested but
unnecessary packages.

Those two fixes hopefully makes travis builds successful again.

Props to @clhunsen for experimenting with the parameters and proposing
the first part of this commit regarding the `verbose` and `quiet`
parameters of the `install.packages` function.

This commit addresses parts of se-sic#161.

Signed-off-by: Thomas Bock <[email protected]>
bockthom added a commit to bockthom/coronet that referenced this issue Nov 8, 2020
This is related to issue se-sic#161.

Signed-off-by: Thomas Bock <[email protected]>
@bockthom bockthom mentioned this issue Nov 8, 2020
6 tasks
@bockthom
Copy link
Collaborator

bockthom commented Nov 8, 2020

I am happy to announce: The current build problems with Travis CI are fixed! (as soon as the corresponding PR #183 will be merged.)
In particular, the log length of package installations is reduced (thanks to @clhunsen) and not needed (so-called "suggested" dependencies) packages are not installed any more.

@bockthom bockthom mentioned this issue Dec 2, 2020
@bockthom
Copy link
Collaborator

bockthom commented Feb 1, 2021

At the moment, we again have some problems in our test pipeline on R 3.3.3, as has been pointed out in PRs #193 and #194. The problem is the following:

Package sqldf imports package RSQLite, which in turn imports package memoise. Last Tuesday (2021-01-26), a new version of package memoise was released (which explains why the pipeline still ran successfully at the beginning of last week). The new release of memoise has introduced a new dependency to package cachem (this dependency was not present before). Package cachem imports package fastmap (which is also new in our dependency tree). And now we are close to the failing builds: The installation and compilation of package fastmap ends up in an "internal compiler error: Segmentation fault" for R 3.3.3. There is no segmentation fault for more recent R versions (>= 3.4), and also no segmentation fault for the even earlier versions R 3.3.1 and R 3.3.2. So only R 3.3.3. is the problem.

After some additional test runs, I identified that the described problem is not related to fastmap, but potentially is a bug in the compiler which is installed in the docker image r-base:3.3.3: This image uses g++ 6.3.0-9, whereas the docker images of the preceding and succeeding R versions use another g++ version. Hence, we have to deal with the docker image and not with the packages. Therefore, in the following, I provide the options we have:

0. Create a docker image which uses R 3.3.3 and another g++ version than 6.3.0-9. This is not really an option, as we won't maintain docker images. We rather should rely on some existing images to reduce maintenance effort.

  1. Use another docker image to cover R 3.3. For example, we could use R 3.2, as the corresponding r-base:3.3.2 docker image uses another g++ version.
  2. Completely remove the support for R 3.3 (then we would have to update several README files and paper websites...)
  3. There is also dedicated docker image which was created for reproducible builds on fixed R versions, such as rocker/r-ver:3.3.3. This docker image only allows dependencies which have been available at the time of R 3.3.3 being released and maintained. For example, using this docker image, package memoise is still installed in the old version which was prevailing until last week. Therefore, it does not even install fastmap. However, we can circumvent this by clearly stating repos = "https://cloud.r-project.org" in our install.R to install recent package versions. However, there are also additional pitfalls for which I don't have found a solution: rocker/r-ver:3.3.3 uses gcc/g++ versions where c++11 mode is not enabled. Hence, installing recent versions of packages such as RcppArmadillo is not possible. As we have problems when installing old package versions and also problems when installing recent package versions, I did not find any reasonable solution which meets our requirements. Hence, the third option was worth a try but was not successful so far.

Which option would you choose? Your comments are welcome 😉

@hechtlC
Copy link
Contributor

hechtlC commented Feb 2, 2021

Sounds like the first option would be the easiest and fastest variant. If you think that this is sustainable and reliable (i.e., we don't have to look for another solution every two weeks) then I would prefer that.

@bockthom
Copy link
Collaborator

bockthom commented Feb 2, 2021

Sounds like the first option would be the easiest and fastest variant. If you think that this is sustainable and reliable (i.e., we don't have to look for another solution every two weeks) then I would prefer that.

Yes, for the moment, this is the easiest and fastest variant. Regarding sustainability and reliability, we cannot really make a statement how this will behave in the future, as new package versions can introduce new dependencies, for which we don't know whether and how they will support old R versions. It can always happen that a new package requires a certain R version. But at the moment, all the R packages we use in our tests are available for all the R versions in our test pipeline (Note that markovchain is not available since quite a long time, but we don't use this package in our tests.)

I would suggest to just go with the first option and use the rbase:3.3.2 docker image––we just need to change one character in our .drone.yml––and use this one unless new problems occur, hopefully far away. If it happens that new problems with package updates and R 3.3 occur in the future, then we still can decide whether to quit the support for R 3.3, which also can be done easily by removing a few lines from our .drone.yml and updating serveral README files.

If there are no further objections, I will provide a patch to adjust the .drone.yml, test it on my own fork, and if it works, send it to @nlschn tomorrow so that he can apply it to PR #193 to get the green check mark for the test pipeline again 😄

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

No branches or pull requests

3 participants