Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some cmalign subprocess udpates #8

Merged
merged 6 commits into from
Jan 30, 2014
Merged

some cmalign subprocess udpates #8

merged 6 commits into from
Jan 30, 2014

Conversation

crosenth
Copy link
Member

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

@cmccoy
Copy link
Member

cmccoy commented Jan 30, 2014

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
Copy link
Member Author

@cmccoy
Copy link
Member

cmccoy commented Jan 30, 2014

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

@crosenth
Copy link
Member Author

done

@cmccoy
Copy link
Member

cmccoy commented Jan 30, 2014

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

@cmccoy cmccoy merged commit dde6516 into fhcrc:master Jan 30, 2014
@crosenth
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants