{bio}[GOMPI-2020a] UShER 0.4.1 created. #13694
Conversation
…clude external TBB libs, removed libtbb_preview.so.2 and replaced with libtbb.so.2, TBB-2021.3.0 misses currently tbb/pipeline.h
sassy-crick
changed the title
|
What I have not included is this (taken from here install faToVcfrsync -aP rsync://hgdownload.soe.ucsc.edu/genome/admin/exe/linux.x86_64/faToVcf . |
sassy-crick
changed the title
There was a problem hiding this comment.
@sassy-crick I would split the patch into two patches because they address different things that aren't connected. Also the usual name for patches is like %(name)s-%(version)s_fix-something.patch so in your case it's something like UShER-0.4.1_fix-isnan.patch and UShER-0.4.1_disable-internal-tbb.patch or so.
|
@SethosII I renamed the patch, that slipped through. I also had some styled problems which oddly enough slipped through as well. |
| ('Boost', '1.74.0'), | ||
| ('Python', '3.8.6'), | ||
| ('zlib', '1.2.11'), | ||
| ('Kent_tools', '411'), |
There was a problem hiding this comment.
This doesn't depend on Kent tools at all. It only uses one specific tool in one example workflow file, which is a file that we don't even install. So there is literally not a single reference to this software anywhere.
There was a problem hiding this comment.
I beg to differ. The installation instructions clearly include faToVcf which is part of the the Kent_tools set. Anybody who is using UShER from the GitHub page thus expects this program to be around when installing UShER via EasyBuild. So for me this is an extension similarly to the modules we are installing in R. So from that angle I think it makes much sense to install that. This is particularly true as ''UShER'' is part of the pagnolin package which is widely used for Covid-19 PCR tests. It does not help to exclude this vital program, which is needed to convert one file format into another one.
I agree with the part of how they included it is not the best way, but I disagree on the part regarding dependency. In order to use the full potential of the program, and in order how it is used in praxis, we should leave the Kent_tool in it as a dependency.
There was a problem hiding this comment.
It is only referenced in
- A snakefile that we don't even install. https://github.com/yatisht/usher/blob/master/workflows/Snakefile#L82
- A snakefile that requires (according to their own instructions) to use conda, which, of course won't be compatible with EB regardless.
Nothing that gets installed here has any connection to the other tools whatsoever.
| @@ -0,0 +1,43 @@ | |||
| Patch to correct CMakeLists.txt so external TBB will be used instead of internal build. | |||
| Patch to convert innan to std::isnan | |||
There was a problem hiding this comment.
Please do these in separate patches. Also, you don't actually need to change the std::isnan for 10.3.0 at least.
Also the rest of the changes are unncessary. Using find_package works fine. cf. #13708
There was a problem hiding this comment.
I am happy to split the patch in two, no worries here.
I cannot use 10.3.0, simply because of the problem I am having with tbb-2021.3.0 and the missing header file so I am stuck with the older version where we only have the 10.2.0 version of gcc.
I am not sure where the std::isnan crept in. I cannot reproduce it any more so I removed that patch, only leaving the patch for CMake.
I had problems with the CMake patch when I tried that method manually which caused the long discussion thread.
The patch I came up with works also outside EasyBuild, which I see as an advantage specially when debugging programs. In the end, both work.
There was a problem hiding this comment.
I have removed the usher-0.4.1-isnan-CMake.patch and replaced it with the usher-0.4.1-CMake.patch one which addresses the CMakeList.txt issue we both noted, and solved differently.
|
Closing in favour of #13708, as I prefer the patch there. |
(created using
eb --new-pr)