Use set operations for unique atom detection in HybridTopologyFactory#1911
Closed
kokishimada wants to merge 2 commits intoOpenFreeEnergy:mainfrom
Closed
Use set operations for unique atom detection in HybridTopologyFactory#1911kokishimada wants to merge 2 commits intoOpenFreeEnergy:mainfrom
kokishimada wants to merge 2 commits intoOpenFreeEnergy:mainfrom
Conversation
Replace linear-scan loops with set difference operations in _set_mappings for finding unique atoms and environment atom maps. Also use dict comprehension for _env_old_to_new_map construction.
Reuse the result already stored in new_bond_parameters instead of calling _find_bond_parameters a second time with the same arguments.
|
No API break detected ✅ |
Member
|
Hi @kokishimada - thanks for opening this PR, this is very interesting. That being said, there is already some work towards speeding up the HTF being done here: #1858 It's possible that we would go for that PR instead, but I will let @jthorton weigh in. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HybridTopologyFactory._set_mappingsfor finding unique atoms_env_old_to_new_mapconstructionTest plan
HybridTopologyFactorypass_unique_old_atomsand_unique_new_atomsproduce the same sorted results as before