Skip to content

Fix arithmetic crystal class for -6m2P and -62mP#367

Merged
lan496 merged 1 commit intospglib:developfrom
lan496:fix-arithmetic-62mP
Dec 4, 2023
Merged

Fix arithmetic crystal class for -6m2P and -62mP#367
lan496 merged 1 commit intospglib:developfrom
lan496:fix-arithmetic-62mP

Conversation

@lan496
Copy link
Copy Markdown
Member

@lan496 lan496 commented Nov 15, 2023

Arithmetic crystal classes for P-6m2 (No. 187), P-6c2 (No. 188), P-62m (No. 199), and P-62c (No. 190) seem to be opposite.

char arithmetic_crystal_class_symbols[74][7] in arithmetic.c looks to follow the order in IUCr's dictionary.
Then, arithmetic number 56 is -62mP, and 57 is -6m2P.
Thus, the arithmetic crystal class for P-6m2 (No. 187) is -6m2P (57) not -62mP (56).

After this PR, these arithmetic crystal classes for 187-190 are fixed as follows:

for hall_number in [481, 482, 483, 484]:
   ...:     data = get_spacegroup_type(hall_number)
   ...:     print(data)

{'number': 187, 'international_short': 'P-6m2', 'international_full': 'P -6 m 2', 'international': 'P -6 m 2', 'schoenflies': 'D3h^1', 'hall_number': 481, 'hall_symbol': 'P -6 2', 'choice': '', 'pointgroup_international': '-6m2', 'pointgroup_schoenflies': 'D3h', 'arithmetic_crystal_class_number': 57, 'arithmetic_crystal_class_symbol': '-6m2P'}
{'number': 188, 'international_short': 'P-6c2', 'international_full': 'P -6 c 2', 'international': 'P -6 c 2', 'schoenflies': 'D3h^2', 'hall_number': 482, 'hall_symbol': 'P -6c 2', 'choice': '', 'pointgroup_international': '-6m2', 'pointgroup_schoenflies': 'D3h', 'arithmetic_crystal_class_number': 57, 'arithmetic_crystal_class_symbol': '-6m2P'}
{'number': 189, 'international_short': 'P-62m', 'international_full': 'P -6 2 m', 'international': 'P -6 2 m', 'schoenflies': 'D3h^3', 'hall_number': 483, 'hall_symbol': 'P -6 -2', 'choice': '', 'pointgroup_international': '-6m2', 'pointgroup_schoenflies': 'D3h', 'arithmetic_crystal_class_number': 56, 'arithmetic_crystal_class_symbol': '-62mP'}
{'number': 190, 'international_short': 'P-62c', 'international_full': 'P -6 2 c', 'international': 'P -6 2 c', 'schoenflies': 'D3h^4', 'hall_number': 484, 'hall_symbol': 'P -6c -2c', 'choice': '', 'pointgroup_international': '-6m2', 'pointgroup_schoenflies': 'D3h', 'arithmetic_crystal_class_number': 56, 'arithmetic_crystal_class_symbol': '-62mP'}

Actually, I think the order of arithmetic crystal classes for -62mP and -6m2P is strange: other cases like -42mP and -4m2P are ordered the same as space group types.

@lan496 lan496 requested a review from atztogo November 15, 2023 11:07
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8fab0b0) 83.68% compared to head (1fbdacc) 83.68%.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop     #367   +/-   ##
========================================
  Coverage    83.68%   83.68%           
========================================
  Files           24       24           
  Lines         8144     8144           
========================================
  Hits          6815     6815           
  Misses        1329     1329
Flag Coverage Δ
c_api 76.45% <ø> (ø)
fortran_api 56.29% <ø> (ø)
python_api 80.54% <ø> (ø)
unit_tests 1.24% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lan496
Copy link
Copy Markdown
Member Author

lan496 commented Dec 2, 2023

@atztogo Can you review this PR when you have time?

atztogo
atztogo approved these changes Dec 3, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@atztogo atztogo left a comment

Choose a reason for hiding this comment

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

I forgotten to approve this. Please always call me in this way any number of times.

@lan496 lan496 enabled auto-merge December 3, 2023 02:30
@lan496 lan496 force-pushed the fix-arithmetic-62mP branch from 1fbdacc to 9392fe5 Compare December 4, 2023 05:55
@lan496 lan496 merged commit 163f143 into spglib:develop Dec 4, 2023
@lan496 lan496 deleted the fix-arithmetic-62mP branch December 4, 2023 10:32
@Porkepix Porkepix mentioned this pull request Dec 8, 2023
Merged
@LecrisUT LecrisUT added this to the 2.2 milestone Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atztogo atztogo atztogo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.2

Development

Successfully merging this pull request may close these issues.

4 participants

@lan496 @codecov-commenter @atztogo @LecrisUT