Fix swapped arithmetic crystal class numbers for space groups 187-190 - #398
Merged
Conversation
Per International Tables for Crystallography, Volume C (2004), Table 1.4.2.1, arithmetic crystal class -6m2P is No. 56 and -62mP is No. 57. Moyo had the numbers reversed, so space groups 187/188 reported arithmetic number 57 and 189/190 reported 56. The internal number<->symbol linkage was consistent, so identification results and the reported symbols were already correct; only the exposed numbers change. Same fix as spglib/spglib#632. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moyo has the same data bug as spglib, fixed there in spglib/spglib#632: the arithmetic crystal class (ACC) numbers 56 and 57 are swapped relative to International Tables for Crystallography, Volume C (2004), Table 1.4.2.1.
The swap was applied consistently across the internal tables, so symmetry identification and the reported ACC symbols were already correct; only the ACC numbers exposed via
MoyoDataset/moyopy.SpaceGroupType.arithmetic_numberwere wrong.Changes:
moyo/src/data/arithmetic_crystal_class.rs: swap the symbols so 56 =-6m2Pand 57 =-62mPmoyo/src/data/hall_symbol_database.rs: Hall entries 481/482 (SG 187/188) now map to ACC 56, and 483/484 (SG 189/190) to ACC 57moyo/src/data/point_group.rs:PointGroupRepresentative::from_arithmetic_crystal_classmaps 56 -> Hall 481 and 57 -> Hall 483CHANGELOG.md: Unreleased entryTest plan
test_arithmetic_crystal_class_for_sg_187_to_190asserting the ACC number and symbol for every Hall entry of SG 187-190 against Table 1.4.2.1 (fails before the fix, passes after)cargo test -p moyopasses (161 unit + integration tests)just prekpasses🤖 Generated with Claude Code