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

Fix antisite generation in mixed-valence systems #210

Merged
merged 5 commits into from
Jan 18, 2025

Conversation

kavanase
Copy link
Contributor

Context: SMTG-Bham/doped#96

Flagged by @alexsquires, for mixed-valence systems AntisiteGenerator gives X_X substitutions. Using sets instead of lists for determining the elements in the structure fixes this issue (see discussion in linked issue). Added a test for this behaviour too, and confirmed it fixes the downstream issue noted in doped

@kavanase
Copy link
Contributor Author

Btw the (previously) failing test here is unrelated to this PR.
Seems to be some small numerical noise in the test_ccd.py results (due to some dependency update I guess?), have fixed this by loosening the relative tolerance to 1e-4 here which is perfectly good.

@kavanase
Copy link
Contributor Author

The other failing test now in the notebooks is again unrelated, something to do with the Freysoldt correction with sxdefectalign

@jmmshn
Copy link
Collaborator

jmmshn commented Jan 18, 2025

@kavanase, sorry for missing this!

Looking at it now.

@jmmshn
Copy link
Collaborator

jmmshn commented Jan 18, 2025

Again sorry for the late reply. I have shifted focus to some functional materials at MS over the past few months but I'll try to tweak my github settings to I catch these alerts better.

Thanks for the update on this.
I think this is fine. I'm still a bit on the fence about using formal oxidation states during defect creation so decision making in this part of the code is pretty arbitrary.

I'll merge and release soon. @tschaume thanks for brining this to my attention!

@jmmshn jmmshn merged commit 722b987 into materialsproject:main Jan 18, 2025
3 of 5 checks passed
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