search

SystemsGenetics / KINC

Knowledge Independent Network Construction
MIT License
11 stars 4 forks source link

Added 'bin' directory, reorganized scripts #135

Closed bentsherman closed 4 years ago

bentsherman commented 4 years ago

This PR is intended to reorganize the scripts in the scripts folder to clarify which ones are used in the normal KINC workflow versus which ones are only used internally for development. So far I have moved the R scripts and the 3d viewer script into a separate bin directory. I also tried to rename them to have a kinc- prefix while also not being too long. But it would be good to have people from the Ficklin lab comment on the file naming.

Currently there is an R script and a Python script which each provide various plots for KINC output. Eventually these two scripts should be combined into a single Python script. I was going to attempt this myself but I realized most of the code in the R script is coming from KINC.R, and since it is a long-term goal to eventually move all of KINC.R into KINC or accompanying Python scripts, I figured it would be better to wait.

Before I merge this PR we need to agree on the script names and then update the documentation accordingly. A related idea was to perhaps document the development scripts in a "development" section of the kINC docs.

spficklin commented 4 years ago

Hey Ben, I like the naming. I fixed the kinc-2d-viewer.py to be kinc-3d-viewer.py and I updated the documentation to use the new names. You can merge this

spficklin commented 4 years ago

Oh, one more question. Will these scripts get installed along with the kinc binaries? If so, I think the edits I made to the doc is fine. If not. We might have to adjust the text a bit.

spficklin commented 4 years ago

Related to the other scripts. I think it's fine to leave the documentation page for them. One person did try to use one of the scripts to run a full test of KINC, but I think we can provide a test dataset that would satisfy that urge.

bentsherman commented 4 years ago

@spficklin I added a Makefile that will build KINC and also copy the main scripts to the install directory. It's actually based on a Makefile I've been using for myself for a long time, to make the build process one small command. You can run it like this:

make install -j8 INSTALL_PREFIX=$HOME

That will use 8 cores and install everything to ~/bin. And it assumes ACE is already installed. Does this setup sound good to you? If so could you update the documentation to your liking? And feel free to test the Makefile it should work like a charm.

spficklin commented 4 years ago

Wonderful! Thanks @bentsherman.

By default, a make install in KINC installs kinc and qkinc into the /usr/local/bin directory. Should we have the scripts installed there as well rather than ~/.bin?

bentsherman commented 4 years ago

@spficklin That was just for that example, by default INSTALL_PREFIX is /usr/local so if you don't specify it then everything will be installed there. So you can use INSTALL_PREFIX to install everything to whatever directory you want.

spficklin commented 4 years ago

Okay, great. I think this is ready for merging unless you have something else to update here.

spficklin commented 4 years ago

Oh... one request. I don't feel comfortable running sudo make install for the compilation part. I think that comes from my systems admin days. I trust our software but others might not. So, can we adjust this so that users can run a simple make -j 8 and then they can run a sudo make install separate?

bentsherman commented 4 years ago

@spficklin Yes the Makefile supports that as is. I'm going to go ahead and merge.