Update numpy and SuiteSparse to use scikit-umfpack (REVIEW)#718
Conversation
sbecuwe
commented
Oct 16, 2015
- update numpy's site.cfg when SuiteSparse is available
- update SuiteSparse's LD_LIBRARY_PATH to make libamd.so and libumfpack.so available
- update numpy's site.cfg when SuiteSparse is available - update SuiteSparse's LD_LIBRARY_PATH to make libamd.so and libumfpack.so available
|
Automatic reply from Jenkins: Can I test this? |
|
Jenkins: ok to test |
There was a problem hiding this comment.
move this above the if and use if suitesparseroot:
also, please use the 'official' name SuiteSparse (get_software_root will convert it to uppercas to check $EBROOTSUITESPARSE)
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1294/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
- clean up rewrite of update of site.cfg for scikit-umfpack
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1295/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
There was a problem hiding this comment.
suggestion to clean this up (way more readable this way imho):
extrasiteconfig += '\n'.join([
"[amd]",
"library_dirs = %s" % os.path.join(amddir, 'Lib'),
"include_dirs = %s" % os.path.join(amddir, 'Include'),
"amd_libs = amd",
"[umfpack]",
"library_dirs = %s" % os.path.join(umfpackdir, 'Lib'),
"include_dirs = %s" % os.path.join(umfpackdir, 'Include'),
"umfpack_libs = umfpack",
])- clean up rewrite of update of site.cfg for scikit-umfpack (part two)
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1296/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
There was a problem hiding this comment.
add the os.path.exists check I mentioned in a previous remark, just as a safe guard in case something changes in future SuiteSparse versions?
There was a problem hiding this comment.
@boegel What do you mean with "ping on this"?
There was a problem hiding this comment.
I mean to ask @sbecuwe if he's going to tackle my remark, since it's blocking this PR.
There was a problem hiding this comment.
@boegel If the remark is about "add the os.path.exists check", where did you mention it?
There was a problem hiding this comment.
In a remark that is now hidden, can't seem to find it myself anymore (maybe something went wrong with submitting it urgh).
Basically, I would make very sure that these directories are there:
if not os.path.exists(amddir) or not os.path.exists(umfpackdir):
raise EasyBuildError("Expected SuiteSparse subdirectories are not both there: %s, %s",
amddir, umfpackdir)- added check for existence of directories in site.cfg part
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1297/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Easyblocks unit test suite FAILed. See https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1304/console for more details. Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do. |
|
Jenkins: test this please |
|
Easyblocks unit test suite FAILed. See https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1305/console for more details. Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do. |
|
Jenkins: test this please |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1306/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Pull request currently on hold due to a (possible dependency) problem identified by a user. |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1324/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1362/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
@sbecuwe: is this still on hold? any updates? what issue was uncovered that made you block this? |
|
Blocked because of problems discovered in the process of [https://github.com/easybuilders/easybuild-easyconfigs/pull/2061]. Problems in easyconfig had been solved a month ago, but forgot to release this corresponding easyblock request. |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1425/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
lgtm |
There was a problem hiding this comment.
double docstring (and typo), fixed with hpcuantwerpen#1
@sbecuwe: please merge?
fix docstring in EB_SuiteSparse.make_module_req_guess
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1437/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Going in, thanks @sbecuwe! |
…-umfpack Update numpy and SuiteSparse to use scikit-umfpack (REVIEW)