qiime2 / q2-cutadapt

BSD 3-Clause "New" or "Revised" License
3 stars 18 forks source link

add support for multicore? #49

Closed hyphaltip closed 2 years ago

hyphaltip commented 2 years ago

Improvement Description Is there a reason the -j option for cutadapt v3.0 isn't used which support multicore runs for cutadapt

Current Behavior Single-thread cutadapt (default -j 1) is what current runs

Proposed Behavior add a CLI option to specify a core / thread count to use

thermokarst commented 2 years ago

Hey there @hyphaltip - we already support cores in the trim-single and trim-paired methods, so I imagine you're probably referring to the demux methods? The reason we don't have the multicore exposed for the demux methods is because cutadapt simply didn't support multicore demux at the time these methods were developed. I would love to see this wired up now that cutadapt supports it - are you interested in contributing that change?

hyphaltip commented 2 years ago

yes this is in reference cutadapt demux-paired - I didn't specify sorry.

I would be happy to try - let me see if I can figure out how to model the same cmdline params in the trim-single that would go into this plugin

Its a huge speed boost for this demux

thermokarst commented 2 years ago

Awesome, that would be great! Let us know if you need a hand with anything, we're always happy to chat!

hyphaltip commented 2 years ago

I notice you don't have defaults structured in same way in _demux as you do in _trim - is that just code drift or intentional?

Is this best place to put Qs? I'm not totally sure how to setup a test env for qiime since I'm mostly just running based on an installed system. I can see how to add all this to the plugin but I assume would also need to add something upstream in qiime core code to send in the core/threads request too?

thermokarst commented 2 years ago

I notice you don't have defaults structured in same way in _demux as you do in _trim - is that just code drift or intentional?

It is intentional - the demux methods don't have nearly as many parameters to orchestrate. That pattern in the filter methods is just a nice way to keep a large chunk of values consistent.

I'm not totally sure how to setup a test env for qiime

https://dev.qiime2.org/latest/

hyphaltip commented 2 years ago

Solution on my branch - pull request coming in via #50 now.

ebolyen commented 2 years ago

Fixed in #50