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

Update docstring default for alchemical_pme_treatment #644

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

IAlibay
Copy link
Contributor

@IAlibay IAlibay commented Jan 9, 2023

Description

Tiny one, just realised the docstring didn't reflect the init arguments.

Didn't think it was changelog worthy so I omitted adding anything there, can do so if wanted.

Status

  • Ready to go

change docstring default for alchemical_pme_treatment
@IAlibay
Copy link
Contributor Author

IAlibay commented Jan 9, 2023

Probably worth asking whilst I'm here, I'm currently digging into charge handling for AbsoluteAlchemicalFactory, is the following still correct in the context of #380?

- 'exact' includes also the reciprocal space contribution, but it's
only possible to annihilate the charges and the softcore parameters
controlling the electrostatics are deactivated. Also, with this
method, modifying the global variable `lambda_electrostatics` is
not sufficient to control the charges. The recommended way to change
them is through the `AlchemicalState` class.

Copy link
Contributor

@mikemhenry mikemhenry left a comment

Choose a reason for hiding this comment

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

Looks good! @jchodera will have to answer your question

@ijpulidos ijpulidos modified the milestones: 0.22.1, 0.23.0 Apr 6, 2023
@lhmzhou
Copy link

lhmzhou commented Apr 21, 2023

13 checks seem to stay in a lock state. Anyway to re-trigger from your end?

@mikemhenry mikemhenry enabled auto-merge (squash) December 5, 2023 19:49
Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Merging #644 (57b61a8) into main (0bbef98) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@mikemhenry mikemhenry merged commit cbff4c8 into choderalab:main Dec 5, 2023
11 checks passed
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.

4 participants