search

cctbx / dxtbx

Diffraction Experiment Toolbox
BSD 3-Clause "New" or "Revised" License
2 stars 18 forks source link

Beam polarisation handling #10

Open ndevenish opened 6 years ago

ndevenish commented 6 years ago

Pointed out by @dagewa the other day, in FormatCBFFullPilatus.py we explicitly override the beam polarisation to synchotron values: https://github.com/cctbx/cctbx_project/blob/c8767224cd85600d3cc6c761741299cbac3448e4/dxtbx/format/FormatCBFFullPilatus.py#L54-L59

This was added 'for the moment' in 19a102069c9d480a0a8c2d9c2772806dc876e9f3. Presumably we shouldn't be doing this? Or should be doing this better... @dagewa also added a ticket https://github.com/dials/dials/issues/592 to allow this to be configured, but ignoring any file contents at the FullPilatus level seems to be the wrong way, at least.

Looking at some full-cbf files that came off of IO4, they seem to actually have this written in MiniCBF metadata as 0.990 , and fullCBF's _diffrn_radiation.polarizn_source_ratio as 0.8. Should we be using this/these instead? Are we writing the full CBF's wrong? Lots of questions about this.

dagewa commented 5 months ago

Going back in time a bit here but totally agree, it seems wrong to hardcode this:

https://github.com/cctbx/dxtbx/blob/7615301b279b913dd7794ee59be043aafffecb3b/src/dxtbx/format/FormatCBFFullPilatus.py#L36-L41

graeme-winter commented 5 months ago

Going back in time a bit here but totally agree, it seems wrong to hardcode this:

https://github.com/cctbx/dxtbx/blob/7615301b279b913dd7794ee59be043aafffecb3b/src/dxtbx/format/FormatCBFFullPilatus.py#L36-L41

Hard to disagree with you @dagewa but most Pilatus detectors are at synchrotrons? What is the right default? 0.5? I would be happier to somehow capture which are at synchrotrons and which are not and fork at that point.

dagewa commented 5 months ago

What I meant is that there is no attempt to use actual values written in the file, it is just always hard-coded. I don't have any files to hand to check, but https://github.com/cctbx/dxtbx/issues/10#issue-434741505 suggests that a value is written somewhere.

dagewa commented 5 months ago

To be clear, I'm sure 0.999 as a fallback seems appropriate, but if the file says something else we should trust the file, right? If the file is wrong then then users have the opportunity to override at import anyway.

graeme-winter commented 5 months ago

To be clear, I'm sure 0.999 as a fallback seems appropriate, but if the file says something else we should trust the file, right? If the file is wrong then then users have the opportunity to override at import anyway.

Agreed - and the beamline should write the correct value if they chose to include this. That's not our problem.