SystemsGenetics / GEMmaker

A workflow for construction of Gene Expression count Matrices (GEMs). Useful for Differential Gene Expression (DGE) analysis and Gene Co-Expression Network (GCN) construction
https://gemmaker.readthedocs.io/en/latest/
MIT License
33 stars 16 forks source link

Kallisto exit code 1 #234

Closed spficklin closed 3 years ago

spficklin commented 3 years ago

This PR primarily fixes the issue of Kallisto providing an exit code of 1 when the sample is so bad that no counts can be made. Kallisto still generates output when no counts and the GEM can still be created, but because of the exit code not being zero, Nextflow terminates. The validExitStatus directive is also deprecated in Nextflow. So, this PR provides a fix in the BASH code to catch the exit code if its a 1 and rewrite it as a zero (see the kallisto.sh file).

Additionally, this PR does the following

  1. Updates the documentation for issue #230 and #232 .
  2. Reverses a fix that was applied to try and get GEMmaker to run on Kubernetes, but was not needed. This PR rolls it back.
  3. Fixes the GitHub pull request template.

To test this PR do the following:

Use these run IDs with the Arabidopsis genome (you can follow the GEMmaker docs to generate these files).

SRR1058270
SRR1058271
SRR1058272

The first two won't have any alignments. The third will. If you test these before pulling this PR, then GEMmaker will fail. If you test it after pulling this PR then GEMmaker will not fail and the samples will be in the GEM and have NAs.

github-actions[bot] commented 3 years ago

This PR is against the master branch :x:


Hi @spficklin,

It looks like this pull-request is has been made against the SystemsGenetics/GEMmaker master branch. The master branch on nf-core repositories should always contain code from the latest release. Because of this, PRs to master are only allowed if they come from the SystemsGenetics/GEMmaker dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page. Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

JohnHadish commented 3 years ago

Main Purpose of Pull Request

I was not able to reproduce the described behavior. I got alignment for all three samples, both with Kallisto and Hisat2. I was running on my local machine with alignments to Tair10 Araport11: Kallisto:

SRX398815   SRX398816   SRX398814
AT1G01010.1 4.0 4.0 6.0
AT1G01020.2 7.11888 0.0 0.0
AT1G01020.6 57.8811 51.0    51.0
AT1G01020.1 0.0 0.0 0.0
AT1G01020.3 0.0 0.0 0.0
AT1G01020.4 0.0 0.0 0.0
AT1G01020.5 0.0 0.0 0.0
AT1G03987.1 0.0 0.0 0.0
AT1G01030.1 0.0 0.0 0.0\

Hisat2:

SRX398816   SRX398815   SRX398814
AT1G01010   0   0   0
AT1G01020   37  43  36
AT1G01030   0   8   7
AT1G03993   0   1   0
AT1G01040   37  51  33
AT1G01050   148 151 130
AT1G03997   0   0   0
AT1G01060   12  4   9
AT1G01070   0   20  0

Command used for Kallisto:

nextflow run systemsgenetics/gemmaker -profile singularity -r dev \
    --pipeline kallisto \
    --kallisto_index_path ~/Documents/00-reference/01-Arabidopsis/TAIR10_Araport11.transcripts.Kallisto.indexed \
    --kallisto_keep_data \
    --sras SRA.id

Additional things in PR:

  1. Documentation looks good.
  2. I do not have ready access to Kubernettes, not able to test
  3. In my opinion, the github pull request form is too long, especially when viewed in markdown. Reading someone elses raw markdown takes more time than the average error reporter is willing to put in. I do not think anyone is going to read it, and it will probably just be ignored. We want to make it as simple as possible for the error reporter to report their findings so that we can improve GEMmaker.
    I personally think that it is better to let them write incomplete error reports, and then have us request additional information as needed. I am of the opinion that the pull request template should be something like this:
Thank you for submitting an error report, this helps us improve GEMmaker. 
Please describe your error and the system you are using. If possible, please provide a reproducible example of the error.
Thank you
spficklin commented 3 years ago

I'm sorry. I gave the wrong SRA IDs. Try these:

SRR398006 SRR398007 SRR1058270

You don't need to test the adjustments related to kubernetes. It was a rollback to original code because the attempted fix didn't work. It got merged in accidentally and shouldn't have been.

As per the pull request template, that's an nf-core template.

JohnHadish commented 3 years ago

Confirmed that GEMmaker does not fail on files without alignment:

SRX115355   SRX115356   SRX398814
AT1G01010.1 0   0   6.0
AT1G01020.2 0   0   0.0
AT1G01020.6 0   0   51.0
AT1G01020.1 0   0   0.0
AT1G01020.3 0   0   0.0
AT1G01020.4 0   0   0.0
AT1G01020.5 0   0   0.0
AT1G03987.1 0   0   0.0
AT1G01030.1 0   0   0.0
spficklin commented 3 years ago

Thanks John!