Skip to content

Conformer performance#25

Open
jthorton wants to merge 2 commits intomasterfrom
conformer_performance
Open

Conformer performance#25
jthorton wants to merge 2 commits intomasterfrom
conformer_performance

Conversation

@jthorton
Copy link
Copy Markdown
Contributor

Description

This PR wraps the conformer generation function with a process pool to help speed up the step. By default, the pool will use all available processors but I have also exposed the argument to the CLI.

The time taken to generate conformers for the ~200 jacs set of ligands is reduced from ~6mins to 30s.

Questions

  • are there any drawbacks to allowing this?

Status

  • Ready to go

@jthorton
Copy link
Copy Markdown
Contributor Author

It looks like there might be some broken tests for the compute side of things but the conformer tests are still working.

@j-wags
Copy link
Copy Markdown
Member

j-wags commented Dec 15, 2020

Cool. Thanks, Josh. I'm behind on my testing for conformer gen, so I may add some more tests to this branch to help me sleep at night. I'll go ahead and merge when this is ready!

@jthorton
Copy link
Copy Markdown
Contributor Author

No problem, I also need to explicitly code up a slightly different path when the number of processors is 1 as I got some strange behaviour in testing I should be able to do this tomorrow if you can hold off on the merge.

@j-wags
Copy link
Copy Markdown
Member

j-wags commented Dec 15, 2020

Sounds good. I'll add the tests to master directly, so you'll get them when you update this branch.

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