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.refine _check_scan_range #1025

Closed
dagewa opened this issue Nov 20, 2019 · 5 comments
Closed

dials.refine _check_scan_range #1025

dagewa opened this issue Nov 20, 2019 · 5 comments

Comments

@dagewa
Copy link
Member

dagewa commented Nov 20, 2019

Before this check was introduced (#778) we would occasionally get failures in scan-varying refinement for datasets where there were no reflections at the scan edges.

Now the check is there, but beamline scientists complain that DIALS does not process data that have blank images at the beginning and/or end of the scan.

I can make the check more lenient, but at some point you just start getting a dodgy refined model, which due to the smoother also begins to affect part of the scan that really do have data. Also things like the average unit cell calculated over the scan become nonsense.

Another option would be for dials.refine to trim the scan to match the range of strong reflections, with a border of up to 5°, say. So, for the data set with 7.25° of images with no strong spots at the start dials.refine could warn that scan-varying refinement would be compromised and trim the first 2.25° off.

I'm not sure if dials.refine should be modifying the scan width like this, but in a sense the whole point of the program is to modify the experimental models, so maybe that is okay?

What do people think?

@dagewa
Copy link
Member Author

dagewa commented Nov 20, 2019

Example of "successful" refinement with limited data at the start of the scan:
image

@Anthchirp
Copy link
Member

specific case: xia2/xia2#381

@phyy-nx
Copy link
Member

phyy-nx commented Nov 20, 2019 via email

@dagewa
Copy link
Member Author

dagewa commented Nov 20, 2019

It is conceivable that trimming the scan would fail to integrate some extremely weak spots that would otherwise have been integrated, but refining a scan-varying model against no data is liable to produce garbage anyway (see plot above) in which case the predictions would probably miss the spots.

In this case I'm not using the minimum number of spots per parameter but just looking at the phi values for the first and last strong spot in the dataset and comparing with the scan edges.

@dagewa
Copy link
Member Author

dagewa commented Nov 21, 2019

Following discussion with @graeme-winter about this and ideas in #1014 we are moving towards Scan being the interface to the useful bit of a image sequence for some experiment. In that case:

  1. On import there is one crystal-less experiment with a scan that refers to the full image range because we don't know any better.
  2. We do spot-finding and indexing which potentially splits into multiple experiments depending on the number of lattices found. The scans associated with these experiments still refer to the full imported image range, because the models are global, static models.
  3. Once we move onto scan-varying refinement we start being more precise by matching each scan to the actual useful image range for each crystal. In that case the trimming behaviour is not just a way of getting scan-varying refinement to work, but a way of expressing the real duration of the experiment for each crystal.

This seems a good concept to me. So I'll work on trimming within dials.refine for scan-varying refinement. @graeme-winter suggests to trim right down to the full range of observed strong spots, considering also their shoebox zsize.

dagewa added a commit that referenced this issue Nov 22, 2019
This fixes #1025 and fixes xia2/xia2#381, but I don't like the
implementation within BlockCalculator and will move it.
@dagewa dagewa mentioned this issue Nov 22, 2019
@dagewa dagewa closed this as completed in 731480e Nov 27, 2019
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

No branches or pull requests

3 participants