Feat/agipd fixed gain correction

Overview

This MR builds upon https://git.xfel.eu/gitlab/detectors/pycalibration/merge_requests/438. It enables the AGIPD pre-correction constants retrieval notebook and the correction notebook to use the new fixed gain mode.

For constant retrieval, this means:

• detect the gain mode from slow data (reusing code from https://git.xfel.eu/gitlab/detectors/pycalibration/merge_requests/438)
• use gain mode parameter when asking CalCat for constants
  • for FF and PC constants, the constants are queried without the gain mode parameter for now as requested by @jsztuk
  • don't try to load threshold constants

For correction, this means:

• detect the gain mode from slow data (reusing code from https://git.xfel.eu/gitlab/detectors/pycalibration/merge_requests/438)
• if fixed gain mode, very explicitly disable some correction bools only relevant for adaptive gain mode

For agipdlib.py, this means:

• add gain_mode attribute to AgipdCorrections
• if in fixed gain mode, skip everything to do with thresholds (init, computation)
  • keep the gain array, but simply set all entries based on the gain mode we're in

I'm setting the MR to target the feat/agipd-add-fixed-gain branch of the previous MR to make diffs a bit more reasonable to look at for now. But still, sorry, the diffs are a bit big. Did minor refactoring of adjacent code.

How has this been tested?

I ran the retrieval notebook with these settings:

in_folder = "/gpfs/exfel/d/raw/SPB/202130/p900188"  # path to input data, required
out_folder = "/gpfs/exfel/data/scratch/hammerd/test/fixed-gain-correction-notebook/from-notebook"  # path to output to, required
sequences = [-1]  # sequences to correct, set to [-1] for all, range allowed
modules = [0, 4, 5, 8]  # modules to correct, set to [-1] for all, range allowed
run = 263 # runs to process, required

karabo_id = "SPB_DET_AGIPD1M-1" # karabo karabo_id
karabo_da = ['-1']  # a list of data aggregators names, Default [-1] for selecting all data aggregators

h5path_ctrl = '/CONTROL/{}/MDL/FPGA_COMP' # path to control information
karabo_id_control = "SPB_IRU_AGIPD1M1" # karabo-id for control device
karabo_da_control = 'AGIPD1MCTRL00' # karabo DA for control infromation

cal_db_interface = "tcp://max-exfl017:8015#8045" # the database interface to use

And the correction notebook with these:

in_folder = "/gpfs/exfel/d/raw/SPB/202130/p900188"  # path to input data, required
out_folder = "/gpfs/exfel/data/scratch/hammerd/test/fixed-gain-correction-notebook/from-notebook"  # path to output to, required
sequences = [0] # sequences to correct, set to -1 for all, range allowed
modules = [0, 5] # modules to correct, set to -1 for all, range allowed
run = 263 # runs to process, required

karabo_id = "SPB_DET_AGIPD1M-1" # karabo karabo_id
karabo_da = ['-1']  # a list of data aggregators names, Default [-1] for selecting all data aggregators
receiver_id = "{}CH0" # inset for receiver devices
path_template = 'RAW-R{:04d}-{}-S{:05d}.h5' # the template to use to access data
h5path = 'INSTRUMENT/{}/DET/{}:xtdf/' # path in the HDF5 file to images
h5path_idx = 'INDEX/{}/DET/{}:xtdf/' # path in the HDF5 file to images
h5path_ctrl = '/CONTROL/{}/MDL/FPGA_COMP' # path to control information
karabo_id_control = "SPB_IRU_AGIPD1M1" # karabo-id for control device
karabo_da_control = 'AGIPD1MCTRL00' # karabo DA for control infromation
cal_db_interface = "tcp://max-exfl017:8015#8045" # the database interface to use

# Correction parameters
blc_noise_threshold = 5000 # above this mean signal intensity now baseline correction via noise is attempted
cm_dark_fraction = 0.66 # threshold for fraction of  empty pixels to consider module enough dark to perform CM correction
cm_dark_range = [-50.,30] # range for signal value ADU for pixel to be consider as a dark pixel
cm_n_itr = 4 # number of iterations for common mode correction
hg_hard_threshold = 1000 # threshold to force medium gain offset subtracted pixel to high gain
mg_hard_threshold = 1000 # threshold to force medium gain offset subtracted pixel from low to medium gain
noisy_adc_threshold = 0.25 # threshold to mask complete adc
ff_gain = 7.2 # conversion gain for absolute FlatField constants, while applying xray_gain

# Correction Booleans
only_offset = False # Apply only Offset correction. if False, Offset is applied by Default. if True, Offset is only applied.
rel_gain = True # do relative gain correction based on PC data
xray_gain = True # do relative gain correction based on xray data
blc_noise = True # if set, baseline correction via noise peak location is attempted
blc_stripes = True # if set, baseline corrected via stripes
blc_hmatch = False # if set, base line correction via histogram matching is attempted, not validated and not in use for current version
match_asics = True # if set, inner ASIC borders are matched to the same signal level, to be reviewed
adjust_mg_baseline = True # adjust medium gain baseline to match highest high gain value
zero_nans = True # set NaN values in corrected data to 0
zero_orange = True # set to 0 very negative and very large values in corrected data
blc_set_min = True # Shift to 0 negative medium gain pixels after offset corr
corr_asic_diag = False # if set, diagonal drop offs on ASICs are correted, to be reviewed
force_hg_if_below = True # set high gain if mg offset subtracted value is below hg_hard_threshold
force_mg_if_below = True # set medium gain if mg offset subtracted value is below mg_hard_threshold
mask_noisy_adc = True # Mask entire ADC if they are noise above a relative threshold, to be reviewed
common_mode = True # Common mode correction
melt_snow = True # Identify (and optionally interpolate) 'snowy' pixels, to be reviewed for adaptive only
mask_zero_std = True # Mask pixels with zero standard deviation across train
low_medium_gap = True # 5 sigma separation in thresholding between low and medium gain

TODO: will update description of testing after running a full calibration job

added 1 commit

• 0284bae9 - Additional cleanup, starting to add gain_mode in agipdutils

Compare with previous version

added 1 commit

• ef0a008a - Flake8 and related fixes

Compare with previous version

added 2 commits

• a324b12f - Cleanup
• 80de4713 - Hack: for FG, get some regular constants, disable some corrections

Compare with previous version

This is added as using module index directly as before fails when the modules requested are not a contiguous interval including 0 (I was testing with arbitrary lonely modules). The solution here is not very pretty---ideally, I'd overhaul the use of module name, module index, and karabo da throughout in the future.

added 2 commits

• fdd0338d - Formatting + minor typos
• 56214c75 - Adding gain mode handling to AgipdCorrections

Compare with previous version

added 2 commits

• 5b1d6e47 - Formatting, absolute multiprocessing import, typos
• 945b6568 - Skip melt_snow, remember gain_mode

Compare with previous version

unmarked as a Work In Progress

changed the description

added 1 commit

• 127de360 - Explicitly set read mode on h5py.File

Compare with previous version

added 123 commits

• 127de360...91d5f179 - 111 commits from branch feat/agipd-add-fixed-gain
• 832ad5b3 - Initial changes to use new stuff from base branch
• ded58b81 - Initial refactoring and cleanup
• bd9ec496 - Additional cleanup, starting to add gain_mode in agipdutils
• 4375aa50 - Flake8 and related fixes
• ec2301a2 - Cleanup
• 8119989e - Hack: for FG, get some regular constants, disable some corrections
• 4ae7ae8b - Formatting + minor typos
• f7f03532 - Adding gain mode handling to AgipdCorrections
• 32ec3759 - Formatting, absolute multiprocessing import, typos
• 4628ab14 - Skip melt_snow, remember gain_mode
• 58e90a84 - Explicitly set read mode on h5py.File
• 67fdd550 - Merge branch 'feat/agipd-fixed-gain-correction' of...

Compare with previous version

added 2 commits

• bfa5cb60 - Bump cal_db_interactive version
• 9b74c447 - Merge branch 'feat/agipd-fixed-gain-correction' of...

Compare with previous version

pep8: I guess this is over > 79?

True it is. I think it's the most readable version of that statement, though. Here are some of the options:

# 1: current
            self.shared_dict[i_proc]['t0_rgain'][first:last] = \
                rawgain / t0[cellid, ...]
# 2: moving backslash
            self.shared_dict[i_proc]["t0_rgain"][first:last] = rawgain / \
                t0[cellid, ...]
# 3: one line (coincidentally, black opts for this)
            self.shared_dict[i_proc]["t0_rgain"][first:last] = rawgain / t0[cellid, ...]
# 4: breaking brackets
            self.shared_dict[i_proc]["t0_rgain"][
                first:last] = rawgain / t0[cellid, ...]
# 5: breaking last bracket
            self.shared_dict[i_proc]['t0_rgain'][first:last] = rawgain / t0[
                cellid, ...]
# 6: chaining dots instead like we're in js
            (self.shared_dict
             .get(i_proc)
             .get("t0_rgain")
             [first:last]) = (rawgain / t0[cellid, ...])

Options 5 and 6 are pretty absurd (though 5 gets the closest to the 79 limit without exceeding it). I don't think breaking the line is worth it here in any of the variants, though - skimming through the file, extra linebreaks with following extra indentation makes the code look busier (is it indentation for alignment or due to logic structure?)

# 7
self.shared_dict[i_proc]["t0_rgain"][first:last] = (
    rawgain / t0[cellid, ...]
)

I vote 3

changed this line in version 14 of the diff

I can see that some of the refactors increase the length of lines over 79.

pep8: Limit all lines to a maximum of 79 characters.

EDIT: There is the option to add # noqa in case a line has to be >79 for any reason.

Right, flake8 is quite unhappy. I will put # noqa on the longer lines in agipdlib.py. Unfortunately, the notebooks are a very big diff away from making flake8 happy.

Don't worry about the notebooks. It will be very hard to review and it was never written while considering pep8 rules.

added 1 commit

• aa274057 - Now hopefully applying the correct version of isort

Compare with previous version

added 1 commit

• b7a4c165 - Decreasing verbosity

Compare with previous version

added 1 commit

• b7a4c165 - Decreasing verbosity

Compare with previous version

added 1 commit

• d5c6b942 - Satisfying flake8 for agipdlib.py

Compare with previous version

I suppose all the minor code formatting comes from black or trying to make it flake8 confirm? We'll probably have to go through this pain sooner or later, might as well do it now.

Given the extensive testing, LGTM this now.

mentioned in commit baae2cce

merged