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

Inconsistent reciprocalspace ASU between gemmi and cctbx #33

Closed
JBGreisman opened this issue May 20, 2020 · 3 comments
Closed

Inconsistent reciprocalspace ASU between gemmi and cctbx #33

JBGreisman opened this issue May 20, 2020 · 3 comments

Comments

@JBGreisman
Copy link
Contributor

Summary

As @kmdalton mentioned in #28, we have been implementing a few tests to make sure that our vectorized implementations of certain spacegroup-related operations in reciprocalspaceship are correct. In testing some code that maps Miller indices to the reciprocalspace ASU, we noticed some differences between our "reference implementations" in cctbx and gemmi.

We iterate over spacegroups using sgtbx.space_group_symbol_iterator(), which has 530 spacegroup settings, and use the extended Hermann-Mauguin for instantiating the gemmi.SpaceGroup. We are finding inconsistencies for 47/530 settings. This list of 47 spacegroups can be generated using the example below:

Example

reproduce_sg_inconsistency.py

from cctbx import sgtbx
from cctbx import miller
import gemmi
import numpy as np

def build_emptyMTZ(sg):
    """Build empty gemmi.MTZ"""
    m = gemmi.Mtz()
    m.spacegroup = sg
    m.add_dataset('crystal')
    m.add_column('H', 'H')
    m.add_column('K', 'H')
    m.add_column('L', 'H')
    m.add_column('M/ISYM', 'Y')
    return m
    
def gemmi_map2asu(H, symbol):
    """Map HKLs to reciprocal space ASU using gemmi"""
    xhm = symbol.universal_hermann_mauguin()
    sg = gemmi.SpaceGroup(xhm)
    m = build_emptyMTZ(sg)
    data = np.append(H, np.zeros((len(H), 1)), 1)
    m.set_data(data)
    m.switch_to_asu_hkl()
    h = m.column_with_label('H').array.astype(int)
    k = m.column_with_label('K').array.astype(int)
    l = m.column_with_label('L').array.astype(int)
    return np.vstack([h, k, l]).T
    
def cctbx_map2asu(H, symbol):
    """Map HKLs to reciprocal space ASU using cctbx"""
    sg  = sgtbx.space_group(symbol)
    asu = sgtbx.reciprocal_space_asu(sg.type())
    H_asu = np.vstack([miller.asym_index(sg, asu, i.tolist()).h() for i in H])
    return H_asu
    
hmin,hmax = -5, 5
H = np.mgrid[hmin:hmax+1:1,hmin:hmax+1:1,hmin:hmax+1:1].reshape((3, -1)).T

for symbol in sgtbx.space_group_symbol_iterator():
    xhm  = symbol.universal_hermann_mauguin()
    H_cctbx = cctbx_map2asu(H, symbol)
    H_gemmi = gemmi_map2asu(H, symbol)
    if not np.array_equal(H_cctbx, H_gemmi):
        print(xhm)

Output

Collapsed Output

I 1 2 1
I 1 1 2
I 2 1 1
P 1 n 1
P 1 1 n
P n 1 1
I 1 m 1
I 1 1 m
I m 1 1
A 1 n 1
I 1 a 1
I 1 c 1
B 1 1 n
I 1 1 b
A 1 1 n
I 1 1 a
C n 1 1
I c 1 1
B n 1 1
I b 1 1
I 1 2/m 1
I 1 1 2/m
I 2/m 1 1
P 1 2/n 1
P 1 1 2/n
P 2/n 1 1
P 1 21/n 1
P 1 1 21/n
P 21/n 1 1
A 1 2/n 1
I 1 2/a 1
I 1 2/c 1
B 1 1 2/n
I 1 1 2/b
A 1 1 2/n
I 1 1 2/a
C 2/n 1 1
I 2/c 1 1
B 2/n 1 1
I 2/b 1 1
R 3 :R
R -3 :R
R 3 2 :R
R 3 m :R
R 3 c :R
R -3 m :R
R -3 c :R

Single HKL example:

symbol = sgtbx.space_group_symbols("I 1 2 1") 
hkl = np.array([[-5, -5, 1]])
print(cctbx_map2asu(hkl, symbol)) # [[-5  5  1]]
print(gemmi_map2asu(hkl, symbol)) # [[ 5  5 -1]]

Issue

Is this expected behavior, or should gemmi and sgtbx both be using the same conventions for the ASU?

@wojdyr
Copy link
Member

wojdyr commented May 20, 2020

It's wrong in gemmi.
Thanks for the script that tests everything. I should be able to fix it today.

@wojdyr wojdyr closed this as completed in e73303c May 20, 2020
@wojdyr
Copy link
Member

wojdyr commented May 20, 2020

With the test program it was easy to fix it - by trials and errors :-)
Thanks again!

@JBGreisman
Copy link
Contributor Author

Awesome -- I installed these changes locally, and I can confirm that all spacegroups are passing. Thanks for the quick attention to this!

JBGreisman added a commit to rs-station/reciprocalspaceship that referenced this issue May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants