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

dials.merge: Fix output of SigF, N+, N- in merged.mtz #1522

Merged
merged 3 commits into from
Dec 15, 2020

Conversation

jbeilstenedmands
Copy link
Contributor

For anomalous=True (the default), when merging anomalous Fs to normal Fs,
don't use_internal_variances. This gives an incorrect (overly high) value for SigF by several factors.

Also, the miller index multiplicities were being written as N+,
N- however this was intended to be the redundancy of observations
of each symmetry group, which is fixed here.

For anomalous=True, when merging anomalous Fs to normal Fs,
dont use_internval_variances. This gives an incorrect
(overly high) value for SigF.

Also, the miller index multiplicities were being written as N+,
N- however this was intended to be the redundancy of observations
of each symmetry group, which is fixed here.
@jbeilstenedmands
Copy link
Contributor Author

Note: this does not affect the xia2 pipeline which uses cctbx_FrenchWilson to convert Is to Fs, only manual merging with dials.merge

@codecov
Copy link

codecov bot commented Dec 11, 2020

Codecov Report

Merging #1522 (42394cb) into master (fcfd7b4) will increase coverage by 0.02%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master    #1522      +/-   ##
==========================================
+ Coverage   65.62%   65.65%   +0.02%     
==========================================
  Files         614      614              
  Lines       68964    69059      +95     
  Branches     9522     9559      +37     
==========================================
+ Hits        45256    45339      +83     
- Misses      21868    21879      +11     
- Partials     1840     1841       +1     

Copy link
Contributor

@rjgildea rjgildea left a comment

Choose a reason for hiding this comment

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

I think the term "multiplicity" is generally preferable to "redundancy" - can we use this instead? It would also be more consistent with existing usage within dials:

$ find . -name "*.py" -type f | xargs grep -i multiplicit | wc -l
      93
$ find . -name "*.py" -type f | xargs grep -i redundanc | wc -l  
       7

and xia2:

$ find . -name "*.py" -type f | xargs grep -i multiplicit | wc -l
     101
$ find . -name "*.py" -type f | xargs grep -i redundanc | wc -l 
      12

@jbeilstenedmands jbeilstenedmands merged commit 9a413e2 into master Dec 15, 2020
@jbeilstenedmands jbeilstenedmands deleted the fix_merged_mtz branch December 15, 2020 12:49
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.

None yet

2 participants