Xinglab / rmats-turbo

Other
222 stars 55 forks source link

akward build process #115

Open cmeesters opened 3 years ago

cmeesters commented 3 years ago

Hi,

rmats-turbo has a build routine, which is totally broken into separate parts, which should work together:

Best regards, Christian Meesters

EricKutschera commented 3 years ago

The bamtools header files are referenced in the rmats cython code: https://github.com/Xinglab/rmats-turbo/blob/v4.1.1/rMATS_pipeline/rmatspipeline/nogilbam.pxd#L10 . The developer who originally worked on the cython implementation of rmats no longer works on the project, but my guess is that including the bamtools source code was an easy way to get things working

The makefile in the main rmats directory runs the build commands for bamtools, the rmats statistical model, and the rmats cython code (which is imported by the main rmats.py script). Since rmats is intended to be run on the command line rather than imported by other code, a top-level setup.py doesn't seem necessary

cmeesters commented 3 years ago

my guess is that including the bamtools source code was an easy way to get things working

Indeed, yet in cluster environments, we rather install optimized code (no conda at all) and provide libraries as dependencies. Shipping 3rd party libraries along with any software has an additional disadvantage: Whenever the library developers change anything upstream, the application developer has to reflect that change in the application trunk or risks not to be able to do that, when there is a severe change.

Whether or not there is a global makefile or a setup.py does not really matter. Having both, governed by a master script is a little cumbersome. I'll give you a link to an easyconfig (easybuild is a build framework for HPC systems) on Compute Canada, I am about to submit a similar easyconfig to easybuild, soon: https://github.com/ComputeCanada/easybuild-easyconfigs/blob/computecanada-main/easybuild/easyconfigs/r/rMATS/rMATS-4.1.1-gmkl-2020a.eb As you can see, it looks odd. And there is a patch file. I have one too, telling rmats-turbo to be compiled against OpenBlas and Scalapack instead of BLAS and Lapack. Imposing such requirements in a hard-coded fashion is unnecessary, too.

I hope that my rather blunt remarks, help improving the distribution and reach of your project.

Cheers, Christian Meesters