Opened 2 months ago

Closed 6 weeks ago

#28884 closed enhancement (fixed)

spkg-configure for R

Reported by: dimpase Owned by:
Priority: major Milestone: sage-9.1
Component: build: configure Keywords:
Cc: tmonteil, fbissey, isuruf, charpent, embray Merged in:
Authors: Dima Pasechnik Reviewers: Emmanuel Charpentier
Report Upstream: N/A Work issues:
Branch: 1f4a7e1 (Commits) Commit: 1f4a7e1c13c3cfa01de45683834be33b4e2985b7
Dependencies: #27870 Stopgaps:

Description (last modified by charpent)

a minimum working on standard distros setup.

Note that R does not have a working uninstall, so one needs to manually uninstall by

rm -rf $SAGE_LOCAL/lib/R
rm $SAGE_LOCAL/bin/R*

followed by

./bootstrap
./configure
./sage -f rpy2
make build

and then test. (Or, of course start from make distclean)

Change History (32)

comment:1 Changed 2 months ago by dimpase

TODO: check that versions of R and libR match

comment:2 Changed 2 months ago by dimpase

  • Status changed from new to needs_review

comment:3 Changed 2 months ago by dimpase

  • Description modified (diff)

comment:4 Changed 2 months ago by dimpase

  • Cc tmonteil added

comment:5 Changed 2 months ago by dimpase

  • Cc fbissey isuruf added

comment:6 Changed 2 months ago by fbissey

I guess we could check for matching versions with that kind of stuff

fbissey@moonloop ~ $ pkg-config --modversion libR
3.4.1
fbissey@moonloop ~ $ R --version
R version 3.4.1 (2017-06-30) -- "Single Candle"
Copyright (C) 2017 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under the terms of the
GNU General Public License versions 2 or 3.
For more information about these matters see
http://www.gnu.org/licenses/.

Nice that libR comes with a .pc file.

comment:7 Changed 2 months ago by dimpase

I'll do the TODO as soon as the rest looks OK. I've done similar checks for pari...

comment:8 Changed 2 months ago by charpent

Does this install on top of #27870, includes (i. e. subsumes) #27870, or what ?

comment:9 Changed 2 months ago by charpent

  • Cc charpent added

comment:10 follow-up: Changed 2 months ago by dimpase

  • Dependencies set to #27870

#27870 should have been listed as a dependency, sorry. Anyway, the branch here includes the branch of #27870, so you just need the branch u/dimpase/packages/rconf

comment:11 in reply to: ↑ 10 Changed 2 months ago by charpent

Replying to dimpase:

#27870 should have been listed as a dependency, sorry. Anyway, the branch here includes the branch of #27870, so you just need the branch u/dimpase/packages/rconf

Thanks ! Looking into it...

comment:12 follow-up: Changed 2 months ago by charpent

LGTM:

  • Sage -R works as expected. Superficial testing is OK.
charpent@zen-book-flip:/usr/local/sage-9$ sage -t --long src/sage/interfaces/r.py 
Running doctests with ID 2019-12-15-10-50-43-b4fc9d4c.
Git branch: t/28884/packages/rconf
Using --optional=build,dochtml,dot2tex,fricas,gap_packages,giacpy_sage,libsemigroups,memlimit,python2,sage,sagenb
Doctesting 1 file.
sage -t --long --warn-long 184.3 src/sage/interfaces/r.py
    [257 tests, 5.48 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 5.7 seconds
    cpu time: 5.3 seconds
    cumulative wall time: 5.5 seconds
charpent@zen-book-flip:/usr/local/sage-9$ sage -t --long src/sage/stats/r.py 
Running doctests with ID 2019-12-15-10-51-20-adfb7e0b.
Git branch: t/28884/packages/rconf
Using --optional=build,dochtml,dot2tex,fricas,gap_packages,giacpy_sage,libsemigroups,memlimit,python2,sage,sagenb
Doctesting 1 file.
sage -t --long --warn-long 184.3 src/sage/stats/r.py
    [2 tests, 0.54 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.7 seconds
    cpu time: 0.5 seconds
    cumulative wall time: 0.5 seconds

make ptestalllong underway...

comment:13 Changed 2 months ago by git

  • Commit changed from bbea1946dd5414fd529125086ca2a495e6876c12 to f66ea24ace841024370253cbcc72a3b109daadcc

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e5c91dbupdate pkgconfig to v1.5.1. See trac #28883
1720b38a bare minumim openblas config
925266buse openblas.pc for blas and lapack, don't require atlas is to be used
b89df80move blas handling to build/pkgs/
6341576create cblas.pc link, too, not only blas and lapack
f66ea24spkg-configure for R

comment:14 follow-up: Changed 2 months ago by dimpase

added forgotten commit https://git.sagemath.org/sage.git/commit/?id=634157628ac3f7292e7065eae462bb6b80db7aed from #27870 to this branch (should not matter, review-wise, I guess)

comment:15 Changed 2 months ago by dimpase

  • Description modified (diff)

comment:16 in reply to: ↑ 12 Changed 2 months ago by charpent

  • Cc embray added
  • Description modified (diff)
  • Reviewers set to Emmanuel Charpentier
  • Status changed from needs_review to positive_review

Replying to charpent:

[ Snip... ]

make ptestalllong underway...

ptestalllong gets 6 transient and 3 permanent failures :

File Result P/T
src/sage/plot/animate.py 7 doctests failed T
src/sage/modular/ssmod/ssmod.py 2 doctests failed T
src/sage/misc/latex.py 1 doctest failed T
src/sage/numerical/backends/glpk_backend.pyx 1 doctest failed P
src/sage/repl/load.py 1 doctest failed T
src/sage/tests/gap_packages.py 1 doctest failed P
src/sage/libs/glpk/error.pyx 1 doctest failed P
src/sage/combinat/designs/ext_rep.py 1 doctest failed T
src/sage/databases/findstat.py 1 doctest failed T

All of these have already been reported for previous release(s) of Python 3-based Sage 9 betas, and therefore considered not to be related to this ticket.

==> positive_review

Given the importance of this ticket (a large headache off the shoulders of active Sage-R users...), I recommend tests on other platforms (Mac OS X and its shenanigans, Windows and the coexistence of Windows and Cygwin libraries...).

Last edited 2 months ago by charpent (previous) (diff)

comment:17 in reply to: ↑ 14 Changed 2 months ago by charpent

Replying to dimpase:

added forgotten commit https://git.sagemath.org/sage.git/commit/?id=634157628ac3f7292e7065eae462bb6b80db7aed from #27870 to this branch (should not matter, review-wise, I guess)

Aaaarghhhh...

Another check may be worthy. But maybe after tests on other platforms... Please ask as neeeded.

comment:18 follow-up: Changed 2 months ago by git

  • Commit changed from f66ea24ace841024370253cbcc72a3b109daadcc to 3812f1ca8ecd0c15926c40d214fcb824996a5677
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

3812f1clist more relevant system packages to install on Debian/Fedora

comment:19 in reply to: ↑ 18 Changed 2 months ago by charpent

  • Status changed from needs_review to positive_review

Replying to git:

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

3812f1clist more relevant system packages to install on Debian/Fedora

Similar results on this second pass. positive review. Same recommendation as before.

comment:20 Changed 2 months ago by dimpase

I think we can weaken the version requirements, it still works fine with R 3.4.4 and openblas 0.2.20.

This is what one has on Ubuntu 18.04 on 32-bit machine (arando buildbot) and on Fedora 26.

comment:21 Changed 2 months ago by git

  • Commit changed from 3812f1ca8ecd0c15926c40d214fcb824996a5677 to 6aba5b4cf2073b12a05b19b127f2258111b2daea
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

6aba5b4lower versions of R and openblas

comment:22 Changed 2 months ago by dimpase

  • Milestone changed from sage-9.1 to sage-9.0
  • Status changed from needs_review to positive_review

setting it back to positive review, as it does only change version numbers

comment:23 Changed 2 months ago by dimpase

It works on MacOS 10.13.6 with Homebrew's R and openblas packages.

It doesn't work with R installed with standalone isntaller from R Project, probably as there is some kind of library/compiler incompatibility - they don't build R with Xcode compiler, they use their "own"; more precisely, OpenMP-enabled clang 8, see https://cran.r-project.org/bin/macosx/tools. Somehow I see an error message while building rpy2, attempting to use -fopenmp switch with Xcode's clang.

comment:24 Changed 2 months ago by vbraun

  • Branch changed from u/dimpase/packages/rconf to 6aba5b4cf2073b12a05b19b127f2258111b2daea
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:25 Changed 2 months ago by vbraun

  • Commit 6aba5b4cf2073b12a05b19b127f2258111b2daea deleted
  • Resolution fixed deleted
  • Status changed from closed to new

More stuff that has #28883 merged in

comment:26 Changed 2 months ago by vbraun

  • Status changed from new to needs_review

comment:27 Changed 2 months ago by vbraun

  • Status changed from needs_review to positive_review

comment:28 Changed 2 months ago by vbraun

  • Branch changed from 6aba5b4cf2073b12a05b19b127f2258111b2daea to u/dimpase/packages/rconf
  • Commit set to 6aba5b4cf2073b12a05b19b127f2258111b2daea

New commits:

e5c91dbupdate pkgconfig to v1.5.1. See trac #28883
1720b38a bare minumim openblas config
925266buse openblas.pc for blas and lapack, don't require atlas is to be used
b89df80move blas handling to build/pkgs/
6341576create cblas.pc link, too, not only blas and lapack
f66ea24spkg-configure for R
3812f1clist more relevant system packages to install on Debian/Fedora
6aba5b4lower versions of R and openblas

comment:29 Changed 8 weeks ago by git

  • Commit changed from 6aba5b4cf2073b12a05b19b127f2258111b2daea to 1f4a7e1c13c3cfa01de45683834be33b4e2985b7
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

f74f554update pkgconfig to v1.5.1. See trac #28883
fb8f4e5allow absent zlib.pc
b3a8891a bare minumim openblas config
c031c9euse openblas.pc for blas and lapack, don't require atlas is to be used
b49f79emove blas handling to build/pkgs/
81bf522create cblas.pc link, too, not only blas and lapack
6df5985spkg-configure for R
0bad8b4list more relevant system packages to install on Debian/Fedora
1f4a7e1lower versions of R and openblas

comment:30 Changed 8 weeks ago by dimpase

  • Status changed from needs_review to positive_review

routine rebase over updated #27870

comment:31 Changed 8 weeks ago by dimpase

  • Milestone changed from sage-9.0 to sage-9.1

comment:32 Changed 6 weeks ago by vbraun

  • Branch changed from u/dimpase/packages/rconf to 1f4a7e1c13c3cfa01de45683834be33b4e2985b7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.