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

[BUG] Show disordered sites without gaps #441

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jmmshn
Copy link
Collaborator

@jmmshn jmmshn commented Jan 2, 2025

Show disordered sites without gaps

Currently there are rendering artifacts that results in some weird looking sites when there is partial occupancy.

image

This is due to some weirdness with how the phiStart and phiEnds are being parse after the python part of the code.

If I try to draw a 0.2/0.2/0.2 alloy site then it becomes more obvious that something is wrong.

image

The data at the python level is here:

Fe (20.0% occupancy) (3.000, 3.000, 0.000)
phiStart: 0.00 pi
phiEnd: 0.40 pi
Co (20.0% occupancy) (3.000, 3.000, 0.000)
phiStart: 0.40 pi
phiEnd: 0.80 pi
Li (20.0% occupancy) (3.000, 3.000, 0.000)
phiStart: 0.80 pi
phiEnd: 1.20 pi

Here the three colors should each be 1/5 of the circle and fit together, but it looks like the phi size of these are doubling each time we go to a new species.

Since the starting phi positions of these are still correct we can mostly fix the plots by nesting the spheres inside each other as I do in this PR. but it would be nice to figure out what exactly is going wrong.

@jmmshn jmmshn changed the title show disordered sites without gaps [BUG] Show disordered sites without gaps Jan 2, 2025
@jmmshn
Copy link
Collaborator Author

jmmshn commented Jan 2, 2025

@mkhorton, I check the data being represented in python and cannot find any problems.

The TS code here also looks fine:
https://github.com/materialsproject/mp-react-components/blob/73f1a1b38ccb4021ece5dcf38d472e95d96521dc/src/components/crystal-toolkit/scene/three_builder.ts#L423-L432

So not sure where this problem could be coming from.

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.

2 participants