[REMI] Add and enhance plots, more documentation, clean-up and some deprecation

Description

A collection of minor changes by themselves, aggregated to an MR for simplicty. I tried to be diligent about each commit though and will describe them individually:

• 1994e74d Requested by operators to quickly visualize how the detector is wired (i.e. digitizer channel to detector signal) and discriminated (discrimination settings are set by digitizer channel but conceptualized by detector signal). [1]

• 42371701 Ongoing effort to remove EXtra-remi and proper use of notebook parameters, here code to open DataCollection, calculate time calibration and hide some already deprecated keys in the user YAML file.

• 59cc7816 A more detailed version of the pulse height distribution already added in !822 (merged) to visualize it as a function of TOF. This can help to understand a different detector response for different particles (e.g. a highly charged and/or fast ion typically causes larger analog signals, also visible in example) [2]

• 88ea35b8 More captions and explanatory text all over the place on how to read figures or what's going on.

• d9738483 This figure was too large to fit on a single report page and always clipped. Bonus code styling fix.

• ce238900 Also part of clean-up, these parameters are kind of useless. Why would we have a configuration to break the format? I've seen them in proposal config though, so preserve them for compatibility.

• 47de13a2 Adding figures in between makes a mess of numbers, so use descriptive labels instead.

How Has This Been Tested?

This should work out of the box:

run = 290  # Run ID.
in_folder = '/gpfs/exfel/exp/SQS/202102/p002676/raw'
out_folder = '<someplace>'
calib_config_path = '/gpfs/exfel/exp/SQS/202102/p002676/usr/remi_proc_config.yaml'

Alternatively for report creation:

xfel-calibrate remi correct --in-folder /gpfs/exfel/exp/SQS/202102/p002676/raw --run 290 --out-folder <someplace> --calib-config-path /gpfs/exfel/exp/SQS/202102/p002676/usr/remi_proc_config.yaml

Relevant Documents

[1]

[2]

Types of changes

• New feature (non-breaking change which adds functionality)
• Docs (changes to the documentation)
• Style (formatting changes only, no code changes)
• Refactor (refactoring code with no functionality changes)
• Chore (non-code changes, e.g. comments, readme, grammar, etc...)

Checklist:

Reviewers

@ahmedk @kluyvert

added Waiting for review label

One small suggestion, otherwise this LGTM.

I've only done fairly shallow code review, because it would take me a fair bit of time to dive into it in depth. I see the sample plots you've put in the MR description, so I trust that you've run the code and checked that the results make sense.

the user YAML file

I haven't looked into precisely what this does, but just to check: if we reference an external file which affects calibration, anyone changing that file, or any changes to the path structure, will break reproducibility. If this is important, we can extend the xfel-calibrate machinery to handle parameters like this specially to preserve the config.

Yes, it is clear this breaks reproducibility as of now, but frankly I would prefer the file to be gone (an effort this MR is also part of) and replaced by a proper user-side configuration. I know this project keeps us waiting for way too long by now, but in the case of REMI, it does seem to warrant a complicated system to handle such externalities upstream.

added 2 commits

• ce238900 - Deprecate notebook parameter whether to save datasets, now always enabled
• 47de13a2 - Replace figure numbers by labels

Compare with previous version

changed the description

@kluyvert Rebasing unfortunately removed your comment from the MR (as you put it on the commit), but I applied your suggestions. While I was at it, I also double-checked that all the unused arguments come from the webservice, partly hardcoded (cycle and karabo_da, latter only for darks) and partly from config (cal-db...).

Rebecca just tipped me off that the pulse height distribution looked weird being cut off at 2048. Indeed I had extended its axis to 12 bits in an earlier MR, but forgot to also bin to 12 bits.

added 1 commit

• 6c936e80 - Fix binning range for integrated pulse height distribution

Compare with previous version

Latest change LGTM

Thanks for review

changed milestone to %3.12.0

merged

mentioned in commit 7fb38e5d

removed Waiting for review label