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

Pass match_water parameter to specialised solvation functions #186

Closed
wants to merge 2 commits into from

Conversation

lohedges
Copy link
Contributor

This PR fixes a bug where the match_water parameter wasn't passed from the generic BioSimSpace.Solvent.solvate function through to the specialised functions for the supported water models. I've generalised the test so it uses both function types so that we catch similarly missed parameters in future, e.g. when anything else is added.

Closes #185.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

Suggested reviewers:

@chryswoods

@lohedges lohedges added bug Something isn't working exscientia Related to work with Exscientia labels Oct 18, 2023
@lohedges lohedges requested a review from chryswoods October 18, 2023 15:34
@lohedges
Copy link
Contributor Author

The CI won't currently work since there are no Sire 2023.5.0.dev packages on our conda channel. I'll re-run once the feature_optimise_openmm branch is merged.

@chryswoods
Copy link
Contributor

I'm happy to merge that if you are happy to review?

@lohedges
Copy link
Contributor Author

Yes, no problem. I'll take a closer look tomorrow but I've been using it today without issue.

@xiki-tempula
Copy link
Contributor

Do you mind give me a ping when the fix has been propagated to the main branch please? Thank you.

@lohedges
Copy link
Contributor Author

No problem. This is just waiting on the Sire PR to be merged. I'm going though that now, so should be done today/tomorrow.

@lohedges
Copy link
Contributor Author

Closing since I somehow branched off main rather than devel. This isn't too much of an issue since this is the first change since the release, but I'll re-open with a new branch to do things properly.

@lohedges lohedges closed this Oct 23, 2023
@lohedges lohedges added invalid This doesn't seem right and removed bug Something isn't working exscientia Related to work with Exscientia labels Oct 23, 2023
@lohedges lohedges deleted the fix_185 branch October 23, 2023 18:37
@lohedges
Copy link
Contributor Author

@xiki-tempula: This has now been backported to main.

@xiki-tempula
Copy link
Contributor

Excellent Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] match_water parameter not passed through to solvation function
3 participants