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

Refactor torsiondrive for easy parallel subclass #351

Merged
merged 1 commit into from
Mar 9, 2022

Conversation

jthorton
Copy link
Contributor

@jthorton jthorton commented Mar 4, 2022

Description

This PR refactors the running of the optimizations for the torsiondrive procedure into a function that can be easily overridden by users who would like to implement a parallel version. This avoids users having to deal with the inner workings of the torsiondrives and can instead focus on the distribution of the optimization tasks.

Changelog description

The torsiondrive procedure has been refactored to make it easier for users to implement a parallel version via subclassing and overwriting the _spawn_optimizations method.

cc @SimonBoothroyd

Status

  • Code base linted
  • Ready to go

@codecov
Copy link

codecov bot commented Mar 4, 2022

Codecov Report

Merging #351 (f794a53) into master (aeead6f) will increase coverage by 0.00%.
The diff coverage is 87.50%.

@dotsdl dotsdl self-requested a review March 7, 2022 23:57
Copy link
Collaborator

@dotsdl dotsdl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me @jthorton! Thank you for putting this together!

@dotsdl
Copy link
Collaborator

dotsdl commented Mar 9, 2022

@loriab it looks like Python 3.6 for xtB is blocking us here? Where is the CI running? Does it not exist anymore and need to be removed from required checks?

@loriab
Copy link
Collaborator

loriab commented Mar 9, 2022

I remember seemingly removing xtb 3.6 a couple times and GH not being convinced. go ahead and merge when ready, and I'll take another look at required checks tomorrow.

@dotsdl
Copy link
Collaborator

dotsdl commented Mar 9, 2022

Thanks @loriab! Unfortunately I don't have sufficient permissions on this repo to merge without that check. I can't click the merge button. 😢

@loriab
Copy link
Collaborator

loriab commented Mar 9, 2022

Thanks @loriab! Unfortunately I don't have sufficient permissions on this repo to merge without that check. I can't click the merge button. 😢

See if merge clear now, @dotsdl

@dotsdl dotsdl merged commit 1a7f3cd into MolSSI:master Mar 9, 2022
@dotsdl
Copy link
Collaborator

dotsdl commented Mar 9, 2022

It works! Thanks @loriab!

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.

3 participants