add dorado #256
Closed
add dorado #256
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
jfy133
reviewed
Apr 26, 2024
There was a problem hiding this comment.
Lots of 'unusual' things to what I normally see in nf-core pipelines, but I don't see anything necessarily breaking, except for the licensing issue.
Main thing though you have quite a few modules that appear to only work with conda (not docker), but there is no documentation on this, I think it would be very important to add lots of warnings/checks in the code and also usage documentation warning users that conda won't be possible in many cases
Comment on lines
+29
to
+30
| trim_barcodes=true | ||
| output_demultiplex_fast5 = true |
| tag "$meta.id" | ||
| label 'process_medium' | ||
|
|
||
| container "docker.io/ontresearch/dorado" |
There was a problem hiding this comment.
Just to double check, it is OK to use this license wise?
There was a problem hiding this comment.
And would this work with singularity stilll?
Comment on lines
+18
to
+19
| dorado download --model $dorado_model | ||
| dorado basecaller $dorado_model $pod5_path --device $dorado_device --emit-fastq > basecall.fastq |
There was a problem hiding this comment.
Are there any options a user could theoretically add? Missing ext.args, for example.
| dorado: \$(echo \$(dorado --version 2>&1) | sed -r 's/.{81}//') | ||
| END_VERSIONS | ||
|
|
||
| gzip basecall.fastq |
There was a problem hiding this comment.
This should probably go before the emissions, and should the file be forced to be basecall.fastq for downstream purposes? Otherwise Iw ould recommend using the ${prefix}.fastq system
| label 'process_medium' | ||
|
|
||
| conda "conda-forge::r-base=4.0.3 bioconda::bioconductor-bambu=3.0.8 bioconda::bioconductor-bsgenome=1.66.0" | ||
| container "docker.io/yuukiiwa/pod5:0.2.4" |
Comment on lines
+67
to
+71
| if (workflow.profile.contains('test')){ | ||
| ch_input_path = params.input_path | ||
| } else { | ||
| ch_input_path = Channel.fromPath(params.input_path, checkIfExists: true) | ||
| } |
There was a problem hiding this comment.
I don't really understand this, there is no difference in the way the channel gets taken right?
There was a problem hiding this comment.
These are different.
For the test, I need to get the stage fast5 directory (with many fast5 files) from nf-core/test-dataset, so the input_path is not local, so checkInExist doesn't work
For the user input, there's no staging of the fast5 directory, so checking whether those exist is needed.
There was a problem hiding this comment.
Ah sorry, misread input_path as just input 😅 . I find the testdata set up unusual which is also why tripped me up, but I don't think this is relevant for this PR (also given you've been waiting such a long time)
| "dorado_device": { | ||
| "type": "string", | ||
| "default": "cuda:all", | ||
| "description": "Device specified using '--device'.", |
There was a problem hiding this comment.
Is dorado a particular model of nanopore or something? What is a dorado device?
There was a problem hiding this comment.
this is for specifying what kind of compute one wants to use: cuda:all for all GPUs or cuda:0 for a specific GPU or CPU
Co-authored-by: James A. Fellows Yates <[email protected]>
Co-authored-by: James A. Fellows Yates <[email protected]>
Co-authored-by: James A. Fellows Yates <[email protected]>
Add prefix parameter in nanoplot
Important! Template update for nf-core/tools v2.14.1
|
replaced by #277 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently,
dorado v0.3.2is incorporated into through its docker container. It works without demultiplexing, but doesn't work with demultiplexing withqcatdownstreamI raised an issue on
dorado's github repo requesting for the dorado v0.4.0 to be dockerized (here is the issue). Will not incorporate basecalling and demultiplexing until the dorado v0.4.0 is available on docker hub.I made some changes to basecalling without demultiplexing, where the user can specify the input fast5 directory from the samplesheet for each sample. If a user has multiple sample, then he/she will have to indicate the respective input fast5 directory for those samples.
Here is the run on my machine:
