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

RBS: reported cb_op now correctly maps input to Bravais setting #2105

Merged
merged 5 commits into from
May 31, 2022

Conversation

rjgildea
Copy link
Contributor

@rjgildea rjgildea commented May 5, 2022

Ensure that the reported reindexing operators correctly map the input symmetry to the given Bravais settings, regardless of whether the input symmetry was a primitive or non-primitive setting

Fixes #1280 (and probably #2029)

@codecov
Copy link

codecov bot commented May 5, 2022

Codecov Report

Merging #2105 (41fe2c6) into main (120cd59) will decrease coverage by 0.00%.
The diff coverage is 69.01%.

@@            Coverage Diff             @@
##             main    #2105      +/-   ##
==========================================
- Coverage   80.36%   80.36%   -0.01%     
==========================================
  Files         580      580              
  Lines       65698    65968     +270     
  Branches     9259     9345      +86     
==========================================
+ Hits        52800    53013     +213     
- Misses      10844    10893      +49     
- Partials     2054     2062       +8     

Copy link
Contributor

@graeme-winter graeme-winter left a comment

Choose a reason for hiding this comment

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

Change set looks good - only concern I have is there is an ambiguity in the definition of a no-op from either None or identity -> would probably make sense to settle on either / or?

Also, I suspect there is something you have learned / already knew about, about cb ops, which would be very useful to informally share somewhere if you could think of a way of doing that? A long form comment in the top of bravais_setting code or something would seem legitimate to aid the reader once they get further down. Distinctions between hkl and abc indexing etc.

Overall though changes look good thank you

@rjgildea rjgildea merged commit 37ef448 into main May 31, 2022
@rjgildea rjgildea deleted the rbs-non-primitive branch May 31, 2022 22:15
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.

dials.refine_bravais_settings misleading if input not P1
3 participants