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

Shorthand to exclude regular calibration images #2511

Merged
merged 6 commits into from
Oct 7, 2023
Merged

Conversation

dagewa
Copy link
Member

@dagewa dagewa commented Oct 4, 2023

While working with @aimeon on processing cRED data from Stockholm University, we found that we wanted to exclude every Nth image from the scan, because these are used for calibration during data collection and do not contain diffraction. We could do this by adding exclude_images parameters to dials.find_spots and dials.integrate, but the command-line parameter then becomes rather long.

This PR adds a new parameter exclude_images_multiple, providing a simple shorthand that can be expanded into the appropriate exclude_images command for any data set. For example, for a 517 image data set, with exclude_images_multiple=20, this expansion is performed:

exclude_images=0:20:20,0:40:40,0:60:60,0:80:80,0:100:100,0:120:120,0:140:140,0:160:160,0:180:180,0:200:200,0:220:220,0:240:240,0:260:260,0:280:280,0:300:300,0:320:320,0:340:340,0:360:360,0:380:380,0:400:400,0:420:420,0:440:440,0:460:460,0:480:480,0:500:500

This is printed to the log to be explicit about the expansion.

I toyed with adding this functionality to the existing exclude_images parameter using some new syntax, but it got messy. In the end, seeing as this is for a special case, I thought adding a separate expert_level=2 parameter was a cleaner approach.

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #2511 (774ec41) into main (7cbc053) will decrease coverage by 0.03%.
Report is 1 commits behind head on main.
The diff coverage is 78.04%.

❗ Current head 774ec41 differs from pull request most recent head f678048. Consider uploading reports for the commit f678048 to get more accurate results

@@            Coverage Diff             @@
##             main    #2511      +/-   ##
==========================================
- Coverage   78.83%   78.81%   -0.03%     
==========================================
  Files         609      609              
  Lines       74522    74554      +32     
  Branches    10597    10605       +8     
==========================================
+ Hits        58749    58758       +9     
- Misses      13606    13619      +13     
- Partials     2167     2177      +10     

Copy link
Contributor

@jbeilstenedmands jbeilstenedmands left a comment

Choose a reason for hiding this comment

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

It does seem quite a niche use case, but the implementation is concise and I've ran a test dataset through with these options and everything behaves as expected so looks good to me.
One thing I wanted to check - does the rotation continue smoothly during the calibration images (it must for the current implementation to be correct)? Otherwise we need to think about splitting the data into separate sweeps which would be much more involved.

@dagewa
Copy link
Member Author

dagewa commented Oct 7, 2023

Thanks! I looked the method up. The relevant part is described here: https://journals.iucr.org/j/issues/2018/06/00/yr5038/index.html#SEC5.1 Rotation is continuous, with defocusing to provide a real space crystal image occurring for every N images. The diffraction patterns either side of image N look fine, so it seems this really can be done quickly enough to affect only individual frames from the sweep.

@dagewa dagewa merged commit 11d532d into main Oct 7, 2023
18 checks passed
@dagewa dagewa deleted the exclude_images_multiple branch October 7, 2023 16:27
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