Skip to content

{bio,numlib}[intel/2016b] QTLtools v1.1, Rmath v3.3.1#5361

Merged
boegel merged 3 commits intoeasybuilders:developfrom
sara-nl:20171117183013_new_pr_QTLtools11
Nov 20, 2017
Merged

{bio,numlib}[intel/2016b] QTLtools v1.1, Rmath v3.3.1#5361
boegel merged 3 commits intoeasybuilders:developfrom
sara-nl:20171117183013_new_pr_QTLtools11

Conversation

@casparvl
Copy link
Copy Markdown
Contributor

(created using eb --new-pr)
QTL tools has a dependency on the Rmath shared library. Depending on configuration settings, the Rmath library may be compiled with a regular R installation, but this is not guaranteed (nor is it the case for e.g. R-3.3.1-intel-2016b.eb). The only way to guarantee the dependency is resolved, is to compile Rmath as a standalone library, which is what the attached Rmath EasyConfig does. The R documentation does not list specific dependencies for Rmath as a standalone library; the dependencies in the attached EasyConfig work on both our systems.

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 17, 2017

@casparvl To deal with the style issues, you can easily check locally using eb --check-style *.eb

@casparvl
Copy link
Copy Markdown
Contributor Author

Yeah, silly. I knew some comments where too long and intended to change that before submitting the pul request - and then forgot. Local check is a good tip though, thanks :)
I'll fix these next week.


source_urls = ['https://qtltools.github.io/qtltools/binaries/']
sources = ['QTLtools_%(version)s_source.tar.gz']
checksums = ['9ee0d69d40e3a1827b8805eb98c1bf8a']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@casparvl Please use SHA256 checksums (EasyBuild discriminates between MD5 & SHA256 based on length, see also http://easybuild.readthedocs.io/en/latest/Writing_easyconfig_files.html#checksums).

checksums = ['9ee0d69d40e3a1827b8805eb98c1bf8a']

#Overwrite CXX, CXXFLAG, LIB_FLAGS and LIB_FILES build options from the default makefile, as these use hard-coded compiler & library paths
buildopts = 'CXX="$CXX -std=c++0x" CXXFLAG="$CXXFLAGS" LIB_FLAGS="-lz -lgsl -lmkl -lbz2 -lm -lpthread -lRmath -lhts -lboost_iostreams -lboost_program_options" LIB_FILES=""'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@casparvl You can flesh out the LIB_FLAGS part on a separate line as follows:

buildopts = 'CXX="$CXX -std=c++0x" CXXFLAG="$CXXFLAGS" LIB_FILES="" '
buildopts += 'LIB_FLAGS="-lz -lgsl -lmkl -lbz2 -lm -lpthread -lRmath -lhts -lboost_iostreams -lboost_program_options"'

sources = ['QTLtools_%(version)s_source.tar.gz']
checksums = ['9ee0d69d40e3a1827b8805eb98c1bf8a']

#Overwrite CXX, CXXFLAG, LIB_FLAGS and LIB_FILES build options from the default makefile, as these use hard-coded compiler & library paths
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after # please for readability, and split this across two lines to avoid lines longer than 120 characters

('bzip2','1.0.6'),
('Boost','1.61.0'),
('Rmath','3.3.1'),
('HTSlib','1.4')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use 4 spaces for indent, no tabs, and no need to indent the closed ]

#Overwrite CXX, CXXFLAG, LIB_FLAGS and LIB_FILES build options from the default makefile, as these use hard-coded compiler & library paths
buildopts = 'CXX="$CXX -std=c++0x" CXXFLAG="$CXXFLAGS" LIB_FLAGS="-lz -lgsl -lmkl -lbz2 -lm -lpthread -lRmath -lhts -lboost_iostreams -lboost_program_options" LIB_FILES=""'

moduleclass = 'bio'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be the last line (just for consistency compared to existing easyconfigs)

('PCRE', '8.38'),
('XZ', '5.2.2'),
('zlib', '1.2.8')
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix indent and move this up, right below sources block

('zlib', '1.2.8')
]

moduleclass = 'numlib'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be the last line


#To build Rmath, docs say you need to execute 'make' in src/nmath/standalone
prebuildopts = 'cd src/nmath/standalone;'
preinstallopts = 'cd src/nmath/standalone;'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use preinstallopts = prebuildopts to avoid repetition


moduleclass = 'numlib'

#To build Rmath, docs say you need to execute 'make' in src/nmath/standalone
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please include a pointer to the documentation

preinstallopts = 'cd src/nmath/standalone;'
sanity_check_paths = {
'files': [],
'dirs': [('lib','lib64')]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this more specific?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do
'files': [('lib/libRmath.a', 'lib64/libRmath.a')]
'files': [('lib/libRmath.so', 'lib64/libRmath.so')]
will that check if libRmath.a and so both exist (they should, with this easyconig), and are either located under lib ór lib64? Or can I not repeat a 'files': several times to do an 'and'?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you should use a list with two entries, each a tuple, but you'll need to spread it across two lines to avoid hitting the line length limit (and also, we avoid hardcoding .so, since on MacOS it's .dylib):

sanity_check_paths = {
    'files': [('lib/libRmath.a', 'lib64/libRmath.a'),
              ('lib/libRmath.%s' % SHLIB_EXT, 'lib64/libRmath.%s' % SHLIB_EXT)],
    'dirs': [],
}

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 17, 2017

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
node2080.delcatty.os - Linux centos linux 7.4.1708, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/947a339178556fa6d66c6191bc718b0d for a full test report.

@boegel boegel added this to the 3.5.0 milestone Nov 17, 2017
@boegel boegel added the new label Nov 17, 2017
…ting rules. Reordered some items. Changed checksums to SHA256. Removed BLAS & Lapack libs as they don't seem to be used/linked to libRmath anyway. Added more specific sanity check.
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two more minor details, looks good to go otherwise! :)

# https://cran.r-project.org/doc/manuals/r-devel/R-admin.html#Configuration-options
prebuildopts = 'cd src/nmath/standalone;'
preinstallopts = prebuildopts
sanity_check_paths = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style nit-picking: please include an empty line above :)


sanity_check_paths = {
'files': [],
'dirs': ['bin']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this more specific, by checking for specific binaries (no need to check all if there are a lot of them).

In this case, I'd use:

sanity_check_paths = {
    'files': ['bin/QTLtools'],
    'dirs': [],
}

@easybuilders easybuilders deleted a comment from boegelbot Nov 18, 2017
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 20, 2017

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
node2455.golett.os - Linux centos linux 7.4.1708, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/846997ba6145dce651b2c59b9c6aa308 for a full test report.

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 20, 2017

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
gligar03.gligar.os - Linux centos linux 7.4.1708, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/dad12a628d27ab57a50a15e713a0cf1b for a full test report.

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 20, 2017

Going in, thanks @casparvl!

@boegel boegel merged commit e681102 into easybuilders:develop Nov 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants