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.

Sign up for GitHub

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

Jump to bottom

CRDGEN-272 #92

Merged
merged 7 commits into from
Apr 27, 2021
Merged

CRDGEN-272 #92

merged 7 commits into from
Apr 27, 2021

Conversation

ZontaNicola
Copy link
Collaborator

Changed the weights a bit so that when picking the "central" ring of a system we tend to pick rings that share more atoms with other rings
also now when adding a side ring we try to align it with the already drawn ring that shares the most atoms with it

added a test

@d-b-w
Copy link
Collaborator

d-b-w commented Apr 22, 2021

@ZontaNicola -
Could you add an image to this PR that demonstrates what this accomplishes?

@d-b-w
Copy link
Collaborator

d-b-w commented Apr 26, 2021

@ZontaNicola -
Please add a test function that checks to see if there are any crossed bonds.

@ZontaNicola
Copy link
Collaborator Author

@ZontaNicola -
Could you add an image to this PR that demonstrates what this accomplishes?

perfection itself ;)
Screenshot 2021-04-27 at 11 23 16

rachelnwalker
rachelnwalker approved these changes Apr 27, 2021
View reviewed changes
Copy link
Collaborator

@rachelnwalker rachelnwalker 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 to me! :)

@d-b-w d-b-w merged commit d7728ed into schrodinger:master Apr 27, 2021
rachelnwalker pushed a commit to rachelnwalker/coordgenlibs that referenced this pull request May 4, 2021
@ZontaNicola @rachelnwalker
CRDGEN-272 (schrodinger#92): smarter logic to select parent rings
ee9d089 
Changed the weights a bit so that when picking the "central" ring
of a system we tend to pick rings that share more atoms with other rings.
Since ring systems are built outwards, and the rings with the most
connections are generally "hardest", this reduces the chance of 
rings turning back on themselves.

also now when adding a side ring we try to align it with the
already drawn ring that shares the most atoms with it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rachelnwalker rachelnwalker rachelnwalker approved these changes

@d-b-w d-b-w Awaiting requested review from d-b-w

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ZontaNicola @d-b-w @rachelnwalker