Handle missing pickle file references in expt list

[benjaminhwilliams]

When importing an experiment list from file with check_format=True, don't give up with a FileNotFoundError if a pickle file reference like the imageset hot pixel mask is broken. Instead, log a warning and continue.

Depends on dials/data-files#19 and, once that is merged, a corresponding PR in dials/data.

This is one possible approach to dials/dials#1238. Whether you regard it as a fix or a workaround depends on your point of view.

[phyy-nx]

Hm, to me if a file is missing then I want the program to stop execution. Missing a hot pixel mask is a serious error and I'd expect my integration results to be impacted. I could imaging wanting to override the error, but I'd think that should need an extra phil parameter to whatever program I'm running.

[graeme-winter]

@phyy-nx issue here is that if you are missing this file you can't use the experiments as reference geometry, which is a show stopper when supporting users. Current advice is for them to edit the files which is simply dumb.

Not suggesting that processing without the mask subsequently is OK, but there is useful information in there

Passing a phil parameter umpteen layers down is also probably close to a non-starter as well, as I think this is happening before phil parameters are parsed...

[phyy-nx]

Reference geometry makes sense, but shouldn't it be loaded with check_format=False in that case? See https://github.com/dials/dials/blob/master/command_line/stills_process.py#L319

[rjgildea]

I think I have to agree with @phyy-nx that I'm concerned about (almost) silently ignoring errors such as missing mask files when their absence could have a big impact on data processing in some cases. Users don't read log files at the best of times, and this is especially so in the context of automated pipeline. It's certainly much more of a gorilla than a true fix for dials/dials#1238, and is likely to come back to bite us at some point.

Regarding passing as reference geometry - I'm pretty certain that with check_format=False the presence or absence of the external mask shouldn't be an issue:

$ dials.import $(dials.data get -q centroid_test_data)/centroid_*.cbf
$ dials.find_spots imported.expt output.experiments=strong.expt write_hot_mask=True
$ mv hot_mask_0.pickle hot_mask_0.pickle.moved
$ grep hot_mask strong.expt
$ ls
dials.find_spots.log	dials.import.log	hot_mask_0.pickle.moved	imported.expt		strong.expt		strong.refl
$ dials.show strong.expt
$ dials.show strong.expt 
DIALS (2018) Acta Cryst. D74, 85-97. https://doi.org/10.1107/S2059798317017235
The following parameters have been modified:

input {
  experiments = strong.expt
}

Experiment 0:
Experiment identifier: 5d13258d-dc52-41b8-802f-b69e2b4e7528
...

[rjgildea]

In fact, with this change, I don't see any error in the output of dials.integrate. Following on from the commands above, I ran:

$ dials.index strong.{expt,refl}
$ dials.integrate indexed.{expt,refl}

dials.integrate ran to completion with no indication of any warning or error message as far as I can tell.

[dagewa]

I'd prefer check_format to go away, so that you can import the file just fine for reference geometry etc, but a failure occurs later on at any point when you try to read a value from the mask. Same true for image data in general

[benjaminhwilliams]

I like @dagewa's suggestion, and I note that #187 addresses that. I've found a fix for the immediate user support problem that prompted this (an implicit check_format=True when importing reference geometries in xia2, see xia2/xia2#485). It looks like this PR was more useful as a straw man than as a patch, so I'll join the party in knocking it down.