some cmalign subprocess udpates

fhcrc / deenurp

16S rRNA gene sequence curation and phylogenetic reference set creation

GNU General Public License v3.0

4 stars 3 forks source link

some cmalign subprocess udpates #8

Closed crosenth closed 10 years ago

crosenth commented 10 years ago

quieting logging status messages
piping cmalign output to logging
defaulting cmalign # of cpus to 2
added tmpdir argument for intermediate files

cmccoy commented 10 years ago

Hey Chris - thanks for this. A few thoughts, if it's not too annoying:

Would you mind (re)basing this of fhcrc/master?
Could --tmp be accomplished by setting TMPDIR in the environment? Seems equivalent, other than creating the directory.
I think the default number of CPUs should be the same across cmalign and pplacer subprocesses.

crosenth commented 10 years ago

done (i think)
yes, removed the --tmp argument
I believe the default for pplacer is also 2 (https://github.com/fhcrc/deenurp/blob/master/deenurp/wrap.py#L106)

cmccoy commented 10 years ago

The number of threads is overridden the only place pplacer is called (not that that's the best option); not so for cmalign.

crosenth commented 10 years ago

done

cmccoy commented 10 years ago

More generally, I think it we should either a) keep the parallelism as is (one process per taxon/OTU) or b) split into threads and processes, p concurrent tasks run, each with the option of using t threads. Task-level parallelism (pplacer, cmalign) would use the t threads; total parallelism would be t * p.

cc @nhoffman

crosenth commented 10 years ago

Thanks Connor. So we don't lose your last comment, wanna copy it to Issues and assign it to me or Noah?