-
Notifications
You must be signed in to change notification settings - Fork 48
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 generic structures for topology graphs using single atom representations. #495
Update generic structures for topology graphs using single atom representations. #495
Conversation
Other than that -- barring the change from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM bar comments above
So, the vertices are a single bead, which means that if there is a 'bend' it is because of the geometry of the input topology graph. This is actually most clear in the M24L48 topology graph, where I clearly did not get the math right on the vertex positions. A negligible effect when the building blocks are full chemical structures though. |
Interesting -- i guess leave it then? I assume we dont wanna mess with the topology graph vertices |
Also -- for me -- i do like the fact that there was a non-abstract example of a real cage. It kind of felt more "real" to me. Do you think having both a real and an abstract example is worthwhile? Is it worth checking with other peeps which representation they would find most useful? |
I actually intend to fix this in the future, because ultimately, these small changes will not effect people's work because geometry optimisations should be done after. But future versions should have more symmetrical inputs, it just makes more sense to me.
I think both have their place, but having both may be too much. Perhaps we ask the discord, and maybe you can ask people in the Jelfs group? |
Also, I think the ball and stick mode reminds users that they are not limited to any particular chemistry. |
Yeah -- go ahead and ask on discord and I will press people in person when I see them |
Based on discussions with others, and difficulty in making useful maps between vertex number and atom colours, I have decided to close this PR and issue. |
Related Issues: #431
Requested Reviewers: @lukasturcani
This PR aims to improve the representation of topology graphs by removing artificial chemistry and focussing on the assignment and connectivity of vertices. Hopefully this improves website load times and accessibility to users. All changes are made in the doc strings, and small changes to readability are also attempted for all topology graphs.