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

Add sample_rate or resolution_cutoff options for to_reciprocalgrid #175

Closed
kmdalton opened this issue Aug 23, 2022 · 5 comments · Fixed by #179
Closed

Add sample_rate or resolution_cutoff options for to_reciprocalgrid #175

kmdalton opened this issue Aug 23, 2022 · 5 comments · Fixed by #179
Labels
API Interface Decisions enhancement Improvement to existing feature good first issue Good for newcomers

Comments

@kmdalton
Copy link
Member

Right now you need to specify the number of grid points along h, k, and l when creating a reciprocal space grid of a column in a data set. This is good for flexibility, but it is not very convenient. It would be nice to have the alternative to specify a sample_rate as in gemmi or a resolution_cutoff.

@kmdalton kmdalton added the enhancement Improvement to existing feature label Aug 23, 2022
@JBGreisman JBGreisman added good first issue Good for newcomers API Interface Decisions labels Aug 24, 2022
@dennisbrookner
Copy link
Contributor

As it happens, I had just written this little helper function as a workaround:

def spacing_to_gridsize(spacing, cell):
    """
    Compute the optimal gridsize based on unit cell size and desired spacing

    spacing : float
        Desired (approximate) grid spacing in Angstroms
    cell : gemmi.UnitCell
        Or anything similar with attributes a, b, and c

    NOTE: does not support different spacing in each direction, but in theory could
    """
    gridsize = []
    for dim in [cell.a, cell.b, cell.c]:

        gridsize.append(int(dim // spacing))

    return gridsize

@JBGreisman
Copy link
Member

I think the most reliable implementations would use gemmi for this:

resolution cutoff:

In [1]: ds.cell.get_hkl_limits(0.95) # Grid size should be this *2 to handle negative values
Out[1]: [28, 33, 36]

sampling rate:

In [2]: ds.to_gemmi().get_size_for_hkl(sample_rate=3)
Out[2]: [80, 96, 108]

@kmdalton
Copy link
Member Author

Just a note. sample_rate and resolution_cutoff / dmin both influence grid size but are not mutually exclusive concepts. I think we should support both simultaneously so that

ds.to_reciprocalgrid("F", sample_rate=3., dmin=5.)

produces a 3x oversampled grid from the reflections out to 5A.

Does that make sense? Am I talking crazy?

@JBGreisman
Copy link
Member

I think that's reasonable. That latter case can still be implemented easily in gemmi for first thresholding at the given dmin prior to the to_gemmi() call.

Just to summarize the proposed API change, there will be two mutually exclusive sets of options:

  1. DataSet.to_reciprocalgrid(grid_size=(int, int, int))
  2. DataSet.to_reciprocalgrid(sample_rate=float) or DataSet.to_reciprocalgrid(dmin=float) or DataSet.to_reciprocalgrid(sample_rate=float, dmin=float)

@kmdalton
Copy link
Member Author

i think we can default sample_rate to 3. and dmin to the resolution of the mtz.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Interface Decisions enhancement Improvement to existing feature good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants