Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions test/fixtures/salmon/reads_R1.fastq.gz

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't check out the commit, but can you comment on where these reads were sourced? We want to track the origin our our test data.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if we didn't add these files. We already have FASTQ fixtures - https://github.com/stjudecloud/workflows/blob/main/test/fixtures/fastqs/README.md

The existing test fixtures should be reused (re: #280 , I don't want more LFS files hitting the history )

Git LFS file not shown
3 changes: 3 additions & 0 deletions test/fixtures/salmon/reads_R2.fastq.gz
Git LFS file not shown
3 changes: 3 additions & 0 deletions test/fixtures/salmon/salmon_index.tar.gz

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index here also needs documentation on how it was generated.

Git LFS file not shown
4 changes: 4 additions & 0 deletions test/fixtures/salmon/transcripts.fasta
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
>transcript1
ATGGCGTACGTTAGCATGCATCGATCGTAGCTAGCTAGCATCGATCGTAGCATGCTAGCATGCATCGATCGTAGCATGCTAGC
>transcript2
GCTAGCATCGATGCATGCTAGCTAGCATGCATCGATCGATCGTAGCTAGCATGCTAGCATCGATCGTAGCATGCATCGATCG
257 changes: 257 additions & 0 deletions tools/salmon.wdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
version 1.1

task build_salmon_index {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build task likely needs the decoys mode exposed as that is the recommended way to run with mapping mode.

meta {
description: "Builds a Salmon index from a transcriptome FASTA file, for use in quantification"
outputs: {
salmon_index_tar_gz: "A gzipped TAR file containing the Salmon index files."
}
}

parameter_meta {
transcripts_fasta: "FASTA format file containing the reference transcriptome to index"
index_name: {
description: "Name for the output index, in compressed archive format. The suffix `.tar.gz` will be added.",
group: "Common",
}
use_all_cores: {
description: "Use all cores? Recommended for cloud environments.",
group: "Resources",
}
ncpu: {
description: "Number of cores to allocate for task",
group: "Resources",
}
modify_disk_size_gb: {
description: "Add to or subtract from dynamic disk space allocation. Default disk size is determined by the size of the inputs. Specified in GB.",
group: "Resources",
}
}

input {
File transcripts_fasta
String index_name = "salmon_index"
Boolean use_all_cores = false
Int ncpu = 4
Int modify_disk_size_gb = 0
}

String salmon_index_filename = index_name + ".tar.gz"

Float transcripts_fasta_size = size(transcripts_fasta, "GB")
Int disk_size_gb = ceil(transcripts_fasta_size * 4) + 10 + modify_disk_size_gb

command <<<
set -euo pipefail

n_cores=~{ncpu}
if ~{use_all_cores}; then
n_cores=$(nproc)
fi

salmon index \
-t "~{transcripts_fasta}" \
-i "~{index_name}" \
-p "$n_cores"

tar -czf "~{salmon_index_filename}" "~{index_name}"
>>>

output {
File salmon_index_tar_gz = salmon_index_filename
}

runtime {
cpu: ncpu
memory: "8 GB"
disks: "~{disk_size_gb} GB"
container: "quay.io/biocontainers/salmon:1.9.0--h7e5ed60_0"
maxRetries: 1
}
}

task quant {
meta {
description: "Runs Salmon quant in mapping-based mode to quantify transcript-level expression from RNA-Seq reads, using a pre-built Salmon index"
outputs: {
quant_results_tar_gz: "A gzipped TAR file containing the Salmon quantification output directory, including `quant.sf`."
}
}

parameter_meta {
salmon_index_tar_gz: "A gzipped TAR file containing the Salmon index files. Suitable as the output of the `build_salmon_index` task."
read_one_fastqs_gz: "An array of gzipped FASTQ files containing read one information"
read_two_fastqs_gz: {
description: "An array of gzipped FASTQ files containing read two information. Omit for single-end reads.",
group: "Common",
}
lib_type: {
description: "Salmon library type describing the relative orientation and strandedness of paired reads.",
help: "Use `A` to let Salmon auto-detect the library type — recommended for most users.",
group: "Common",
}
prefix: {
description: "Prefix for the Salmon quantification output. The extension `.tar.gz` will be added.",
group: "Common",
}
num_bootstraps: {
description: "Salmon has the ability to optionally compute bootstrapped abundance estimates.",
help: "This is done by resampling (with replacement) from the counts assigned to the fragment equivalence classes, and then re-running the optimization procedure for each such sample.",
group: "Salmon Options",
}
incompat_prior: {
description: "This parameter governs the a priori probability that a fragment mapping is nonetheless the correct mapping.",
help: "Specifically, this is for a fragment mapping or aligning to the reference in a manner incompatible with the prescribed library type.",
group: "Salmon Options",
}
range_factorization_bins: {
description: "The range-factorization feature allows using a data-driven likelihood factorization.",
help: "This can improve quantification accuracy on certain classes of difficult transcripts.",
group: "Salmon Options",
}
fld_mean: {
description: "Allows the user to set the expected mean fragment length of the sequencing library.",
help: "Since the empirical fragment length distribution cannot be estimated from the mappings of single-end reads, this is only important when running Salmon with single-end reads.",
group: "Salmon Options",
}
fld_sd: {
description: "Allows the user to set the expected standard deviation of the fragment length distribution.",
help: "Since the empirical fragment length distribution cannot be estimated from the mappings of single-end reads, this is only important when running Salmon with single-end reads.",
group: "Salmon Options",
}
seq_bias: {
description: "Passing this flag will enable it to learn and correct for sequence-specific biases in the input data.",
group: "Salmon Options",
}
gc_bias: {
description: "Passing this flag will enable it to learn and correct for fragment-level GC biases in the input data.",
group: "Salmon Options",
}
pos_bias: {
description: "Passing this flag will enable modeling of a position-specific fragment start distribution.",
group: "Salmon Options",
}
use_em: {
description: "Use the \"standard\" EM algorithm to optimize abundance estimates instead of the variational Bayesian EM algorithm.",
group: "Salmon Options",
}
recover_orphans: {
description: "This flag (which should only be used in conjunction with selective alignment), performs orphan \"rescue\" for reads.",
group: "Salmon Options",
}
hard_filter: {
description: "This flag (which should only be used with selective alignment) turns off soft filtering and range-factorized equivalence classes.",
help: "Removes all but the equally highest scoring mappings from the equivalence class label for each fragment.",
group: "Salmon Options",
}
allow_dovetail: {
description: "Dovetailing mappings and alignments are considered discordant and discarded by default.",
help: "If you wish to consider dovetailing mappings as concordant, you can do so by passing this flag.",
group: "Salmon Options",
}
dump_eq: {
description: "If passed, Salmon will write a file in the auxiliary directory, called eq_classes.txt.",
help: "Contains the equivalence classes and corresponding counts that were computed during quasi-mapping.",
group: "Salmon Options",
}
write_unmapped_names: {
description: "Passing this flag will tell Salmon to write out the names of reads (or mates in paired-end reads) that do not map to the transcriptome.",
group: "Salmon Options",
}
use_all_cores: {
description: "Use all cores? Recommended for cloud environments.",
group: "Resources",
}
ncpu: {
description: "Number of cores to allocate for task",
group: "Resources",
}
modify_disk_size_gb: {
description: "Add to or subtract from dynamic disk space allocation. Default disk size is determined by the size of the inputs. Specified in GB.",
group: "Resources",
}
}

input {
File salmon_index_tar_gz
Array[File] read_one_fastqs_gz

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Array[File] read_one_fastqs_gz
Array[File]+ read_one_fastqs_gz

This needs to be non-empty.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adthrasher I think we stopped using non-empty arrays as the resulting WDL is unwieldy ? Or we had a commit adding them and then removing them? I can't remember where we landed on it, but I'm fine without this. If the user doesn't supply any FASTQs, salmon will blow up with an informative error, so 🤷‍♀️

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember the details now. It's something we should probably revisit, though. I'd much rather the WDL fail upfront at analysis because of an empty array than the underlying tool erroring. If WDL doesn't do non-empty arrays well, then we should push for updates to the spec and to the engine(s).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll investigate 🫡
in the mean time, let's follow through with Andrew's original recommendation here and make it non-empty

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CHANGELOG entry points to #240

Array[File]? read_two_fastqs_gz
String lib_type = "A"
String prefix = basename(read_one_fastqs_gz[0], ".fastq.gz")
Int num_bootstraps = 0
Float incompat_prior = 0.0
Int range_factorization_bins = 4
Int fld_mean = 250
Int fld_sd = 25
Boolean seq_bias = false
Boolean gc_bias = false
Boolean pos_bias = false
Boolean use_em = false
Boolean recover_orphans = false
Boolean hard_filter = false
Boolean allow_dovetail = false
Boolean dump_eq = false
Boolean write_unmapped_names = false
Boolean use_all_cores = false
Int ncpu = 4
Int modify_disk_size_gb = 0
}

Array[File] read_twos = select_first([read_two_fastqs_gz, []])

Float read_one_size = size(read_one_fastqs_gz, "GB")
Float read_two_size = size(read_twos, "GB")
Float index_size = size(salmon_index_tar_gz, "GB")
Int disk_size_gb = ceil((read_one_size + read_two_size + index_size) * 3) + 10 + modify_disk_size_gb

command <<<
set -euo pipefail

n_cores=~{ncpu}
if ~{use_all_cores}; then
n_cores=$(nproc)
fi

mkdir salmon_index
tar -xzf "~{salmon_index_tar_gz}" -C salmon_index --strip-components 1

# shellcheck disable=SC2086
# shellcheck disable=SC2086
salmon quant \
-i salmon_index \
-l "~{lib_type}" \
-1 ~{sep(" ", squote(read_one_fastqs_gz))} \
~{if length(read_twos) > 0 then "-2 " + sep(" ", squote(read_twos)) else ""} \
--validateMappings \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default right? This probably needs to be a Boolean input with a true default.

-p "$n_cores" \
--numBootstraps ~{num_bootstraps} \
--incompatPrior ~{incompat_prior} \
--rangeFactorizationBins ~{range_factorization_bins} \
~{if length(read_twos) == 0 then "--fldMean " + fld_mean else ""} \
~{if length(read_twos) == 0 then "--fldSD " + fld_sd else ""} \
~{if seq_bias then "--seqBias" else ""} \
~{if gc_bias then "--gcBias" else ""} \
~{if pos_bias then "--posBias" else ""} \
~{if use_em then "--useEM" else ""} \
~{if recover_orphans then "--recoverOrphans" else ""} \
~{if hard_filter then "--hardFilter" else ""} \
~{if allow_dovetail then "--allowDovetail" else ""} \
~{if dump_eq then "--dumpEq" else ""} \
~{if write_unmapped_names then "--writeUnmappedNames" else ""} \
-o "~{prefix}"

tar -czf "~{prefix}.tar.gz" "~{prefix}"
>>>

output {
File quant_results_tar_gz = prefix + ".tar.gz"
}

runtime {
cpu: ncpu
memory: "16 GB"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does salmon use a consistent amount of RAM or is it dependent on the input and/or transcriptome?

disks: "~{disk_size_gb} GB"
container: "quay.io/biocontainers/salmon:1.9.0--h7e5ed60_0"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we're using such an old version of salmon?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd picked 1.9.0 somewhat arbitrarily. I did try updating to 1.12.1 (the latest release still on the original C++ codebase — 2.0+ is a full Rust rewrite with a different index format, so I avoided that for now), but that specific container build (quay.io/biocontainers/salmon:1.12.1--h017bda4_0) hits a locale::facet::_S_create_c_locale crash during indexing in my test environment — a known class of bug in minimal Docker images missing locale data, unrelated to our WDL logic itself. Reverting to 1.9.0, which runs cleanly and passes both tests. Happy to revisit if you know of a working newer tag, or if this is worth filing upstream with BioContainers.

maxRetries: 1
}
}
23 changes: 23 additions & 0 deletions tools/test/salmon.yaml

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this to use yaml as the extension to match our repository convention?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this should go under tools/test/.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
build_salmon_index:
- name: builds_index_successfully
inputs:
transcripts_fasta:
- salmon/transcripts.fasta
assertions:
outputs:
salmon_index_tar_gz:
- Name: salmon_index.tar.gz

quant:
- name: quantifies_paired_end_reads

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since SE mode is implemented, it should get a test.

inputs:
salmon_index_tar_gz:
- salmon/salmon_index.tar.gz
read_one_fastqs_gz:
- - salmon/reads_R1.fastq.gz
read_two_fastqs_gz:
- - salmon/reads_R2.fastq.gz
assertions:
outputs:
quant_results_tar_gz:
- Name: reads_R1.tar.gz