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

Document indexing strategies #1519

Merged
merged 8 commits into from
Dec 11, 2020
Merged

Conversation

benjaminhwilliams
Copy link
Member

Provide PHIL help for each of the basis vector and lattice search strategies and pass these through to the dials.index PHIL scope. Give a little more info on how each strategy works in the docstring of the corresponding class.

benjaminhwilliams and others added 4 commits December 7, 2020 17:39
Co-authored-by: David Waterman <dagewa@users.noreply.github.com>
Co-authored-by: Richard Gildea <rjgildea@users.noreply.github.com>
Not sure how I managed to commit the file but not its contents...
@codecov
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #1519 (4dd12f9) into master (b36efe5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1519   +/-   ##
=======================================
  Coverage   65.62%   65.62%           
=======================================
  Files         614      614           
  Lines       68965    68970    +5     
  Branches     9529     9529           
=======================================
+ Hits        45257    45261    +4     
- Misses      21866    21867    +1     
  Partials     1842     1842           

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.

Thanks for adding these, I'm sure users will find the additional help strings useful.

As well as being untidy, I'm not convinced the extra whitespace padding and single quotes in the phil_help attributes are even necessary - the following still appears to work:

libtbx.phil.parse("""
%s
.expert_level=1
.help=%s
{}
""" % (entry_point.name, entry_point.load().phil_help[2:-2])).show(attributes_level=1)

fft1d
  .help = "Search for the basis vectors of the direct lattice by performing a"
          "series of 1D FFTs along various directions in reciprocal space."
          "This has a lower memory requirement than a single 3D FFT (the fft3d"
          "method). This method may also be more appropriate than a 3D FFT if"
          "the reflections are from narrow wedges of rotation data or from"
          "stills data."
{
}

i.e. I think we can safely remove the whitespace padding and extra single quotes and it will still work. The default Strategy.phil_help could also be phil_help = None which seems cleaner and more Pythonic.

algorithms/indexing/basis_vector_search/fft1d.py Outdated Show resolved Hide resolved
algorithms/indexing/basis_vector_search/fft1d.py Outdated Show resolved Hide resolved
algorithms/indexing/basis_vector_search/fft1d.py Outdated Show resolved Hide resolved
algorithms/indexing/basis_vector_search/fft3d.py Outdated Show resolved Hide resolved
algorithms/indexing/lattice_search/low_res_spot_match.py Outdated Show resolved Hide resolved
algorithms/indexing/lattice_search/low_res_spot_match.py Outdated Show resolved Hide resolved
algorithms/indexing/lattice_search/low_res_spot_match.py Outdated Show resolved Hide resolved
algorithms/indexing/basis_vector_search/fft1d.py Outdated Show resolved Hide resolved
Copy link
Member

@dagewa dagewa left a comment

Choose a reason for hiding this comment

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

Looks good to me. I added one suggestion for the low res spot match help.

algorithms/indexing/lattice_search/low_res_spot_match.py Outdated Show resolved Hide resolved
@benjaminhwilliams
Copy link
Member Author

As well as being untidy, I'm not convinced the extra whitespace padding and single quotes in the phil_help attributes are even necessary

The default Strategy.phil_help could also be phil_help = None which seems cleaner and more Pythonic.

Oh brilliant, I approve. I thought I tried this in tinkering with PHIL but I clearly did something else wrong and convinced myself it was because I was passing None or because I hadn't properly quoted the help string. Every time I try to use PHIL, I use it wrong somehow, so I can never work out what the rules are.

benjaminhwilliams and others added 2 commits December 11, 2020 10:19
Co-authored-by: Richard Gildea <rjgildea@users.noreply.github.com>
Co-authored-by: David Waterman <dagewa@users.noreply.github.com>
@benjaminhwilliams benjaminhwilliams merged commit e578608 into master Dec 11, 2020
@benjaminhwilliams benjaminhwilliams deleted the document-indexing-strategies branch December 11, 2020 12:14
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

3 participants