Skip to content

Raise error when disordered structure is passed to Transitions #364

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

Merged
merged 4 commits into from
Apr 8, 2025

Conversation

stefsmeets
Copy link
Contributor

This PR changes how disordered species are handled by Transitions. It now raises an error, because the analysis does not work with disordered species. I also added this Transitions.from_trajectory), so it now breaks at the earliest possible stage. I added a function gemdat.utils.remove_disorder_from_species to help 'fix' the structure.

This fixes 2 issues with the current code:

  1. The current code updates sites in-place. This can give unintended side effects
  2. There is better separation of concerns. This changes allows the user to inspect the sites before using the Transitions code. This prevents unwanted changes to the structure and places control for making sure the structure is what they wanted with the users.

Usage:

from gemdat.utils import remove_disorder_from_structure

new_structure = remove_disorder_from_structure(structure)

Closes #343

@stefsmeets stefsmeets merged commit 9c18254 into main Apr 8, 2025
4 checks passed
@stefsmeets stefsmeets deleted the transitions-disorder branch April 8, 2025 06:38
@AstyLavrinenko
Copy link
Contributor

Hi @stefsmeets this function still doesn't address the problem with duplicated sites after setting the occupancies to 1. Could you please fix this.

Also for the error could please change the text from "disorder" to "partial occupancy" as disorder implies ordered and disordered phases while we state here only the existence of all possible for occupation sites which is not necessarily ordered phase.

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.

Dealing with partial occupancies
2 participants