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

Reorganise CIF Import GUI, Molecular Supercell Generation & Configuration Outputs #1679

Merged
merged 28 commits into from
Nov 13, 2023

Conversation

jws-1
Copy link
Member

@jws-1 jws-1 commented Oct 22, 2023

This PR fixes/enhances various parts of the CIF import process. In particular:

  • Fixed bug preventing progression through the wizard.
  • Removed Species Partitioning page, replaced with an Output page where the partitioning can be performed.
  • The Output page gives a preview of the Configuration that will be outputted.
  • Through the Output page, a supercell can be created, the partitioning of the species can be changed and a configuration can be optionally outputted (for the molecular and framework cases).
  • The molecular species that will be created are also listed.
  • Supercell generation has been extended to support molecular species (closes Handle molecular supercell generation #1575).

This definitely needs thorough testing and analysis to ensure that the initial intentions of the code have not been lost!

@jws-1 jws-1 requested a review from trisyoungs October 22, 2023 13:31
@jws-1 jws-1 marked this pull request as ready for review October 22, 2023 16:07
Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Overall I think this puts us exactly where we want to be, but I spotted one significant thing during testing, and that is that when generating a supercell of molecules we don't handle the bonding correctly, or at least we aren't storing and using the "reassembled" molecular fragments in the first place:
image
(This output is from 251754.cif, with a 2,1,1 supercell).

A further small thing is that we default to a framework species on the final output page, but if we know we have molecular species in the system we should default to generating molecular output.

Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

This has highlighted some other things we can do, but I have captured these as separate issues as this PR is already big enough.

@trisyoungs trisyoungs merged commit a1d1b75 into develop Nov 13, 2023
7 checks passed
@trisyoungs trisyoungs deleted the reorganise-cif-import-gui branch November 13, 2023 08:52
rprospero pushed a commit that referenced this pull request Mar 11, 2024
rprospero pushed a commit that referenced this pull request Apr 8, 2024
rprospero pushed a commit that referenced this pull request Apr 9, 2024
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.

Handle molecular supercell generation
2 participants