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

Extend Implementation of Unwrap #2264

Open
NinadBhat opened this issue May 19, 2019 · 4 comments

Comments

@NinadBhat
Copy link
Member

commented May 19, 2019

The current implementation of Group.unwrap(PR #2189) works for bonded structures.
This method should also work for molecules which are part of the same group, e.g., DNA molecules.

Proposed Solution

  • Add keyword reference_point
  • Unwrap molecules such that they are closest to the reference_point

Questions to answered before implementation

  • Is there a more efficient method for unwrapping?
  • Which test cases will be used?

[EDIT]

Current method
I am writing method which shifts a molecule to its periodic image which is closest to the reference point.
So the working will be as follows:
For each molecule in a group

  • make_whole will unwrap the molecule
  • chose_periodic_image will translate the molecule to the periodic image which is closest to the reference point
@jbarnoud

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

@NinadBhat To be able to aswer the question "Is there a more efficient method for unwrapping?" it would be good to write down here what is the method you are thinking about.

@NinadBhat NinadBhat referenced a pull request that will close this issue May 29, 2019

Open

Extending implementation of unwrap #2268

1 of 4 tasks complete
@NinadBhat

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

@jbarnoud I have updated the issue with the current methods. Kindly, have a look.

@richardjgowers

This comment has been minimized.

Copy link
Member

commented May 29, 2019

@NinadBhat make_whole is currently just doing a breadth first search of the molecule's bonds and correcting bond lengths that are wrong as it goes. There's probably some performance that you can squeeze out of it, but I don't think the method itself is wrong/inefficient. I'd focus on getting things implemented then look at performance if you have time later.

@jbarnoud

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

I would be curious about the performance of an alternative algorithm: connecting all the fragments with pseudo bonds, then running make_whole as if it was just one fragment. I do not know what the approach is worth, but it was my first idea when I thought about the issue.

@NinadBhat NinadBhat moved this from Issues to In progress in GSoC 2019 - Wrapping/Unwrapping Jun 17, 2019

@NinadBhat NinadBhat moved this from In progress to Issues in GSoC 2019 - Wrapping/Unwrapping Jun 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.