Skip to content

Conversation

@ndaelman-hu
Copy link
Collaborator

@ndaelman-hu ndaelman-hu commented Aug 22, 2025

  • Introduce base class GlobalSymmetry and rename Symmetry to GlobalCrystalSymmetry.
  • Step away from defining standardized cells (primitive and conventional) to just annotating the cell and symmetry for the system that is being represented. This removes complexity in the normalization and the navigation between the repeating subsections ModelSystem.cell and ModelSystem.symmetry.
  • Make subsections ModelSystem.cell and ModelSystem.symmetry non-repeating.
  • Heavily expand on the description of the crystal symmetry descriptors.
    • Clarify that all are defined in terms of the conventional unit cell.
    • Add lower-dimensional lattices too.

- a single cell and symmetry definition for each model system
- explain the role of these fields among themselves and with subsystems
- Introduce a more appropriate base class `GlobalSymmetry`, from which we define `GlobalCrystalSymmetry`
- Distinguish between short and long Hall symbols (might be reverted)
- Add origin_shift and transformation_matrix to define the conventional cell
- Apply split between cell and symmetry in code
@ndaelman-hu ndaelman-hu self-assigned this Aug 22, 2025
@ndaelman-hu ndaelman-hu added improvement/fix Improvement or fix of a previous feature sprint Issue associated with a sprint labels Aug 22, 2025
@ndaelman-hu
Copy link
Collaborator Author

@lauri-codes Do you see any issues with these changes? I understand that the previous implementation mimics the current results section, but I think that the topology normalizer already does enough constructing the conventional cell.

@lauri-codes
Copy link

Having one section for the symmetry properties makes sense: this is also how the current results section deals with it, and this makes queries easier.

Having only one cell is not a big deal if one of the normalizers calculates the other representations. But like I mentioned in the other PR, if you anyways populate the symmetry section, you will basically automatically also have the primitive and conventional cells available, and the normalizers needs to re-run the symmetry analysis from scratch.

@ndaelman-hu
Copy link
Collaborator Author

Having one section for the symmetry properties makes sense: this is also how the current results section deals with it, and this makes queries easier.

Having only one cell is not a big deal if one of the normalizers calculates the other representations. But like I mentioned in the other PR, if you anyways populate the symmetry section, you will basically automatically also have the primitive and conventional cells available, and the normalizers needs to re-run the symmetry analysis from scratch.

Good point regarding double work. To my understanding, atm topology does indeed run the symmetry analysis again.
I know that the vision has been to move everything to the simulation schema, then copy over relevant parts to results.

If we want MatID to also cover molecules and 1D, as well as magnetic symmetry, then I guess we have some flexibility at which stage we perform the symmetry analysis. If not, we proceed with the "copying" vision.
Some of the recent changes to our schema, though, make it less obvious where new representation (and positions) would be stored. I guess this is something to be figured out with the rest of Area C ( @JFRudzinski ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement/fix Improvement or fix of a previous feature sprint Issue associated with a sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants