Feat/epix dark characterization common mode

[EPIX][DARK] add common mode correction.

Description

• Adds common mode correction to dark characterization notebook, improving the sensitivity to bad pixels.
• Noise and bad pixels maps are calculated independently for each ASIC.

How Has This Been Tested?

• Running the notebook
• Command line "xfel-calibrate EPIX100 DARK"

Relevant Documents (optional)

Types of changes

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

Reviewers

added Waiting for review label

requested review from @ahmedk

assigned to @duarten

unassigned @duarten

added 1 commit

• 8536a23c - Initial commit: Add common mode to Dark Characterization NB

Compare with previous version

Hi @ahmedk, please tag whoever else should be involved in this review. This notebook introduces a lot of changes to the previous NB version. The major component is the introduction of a common mode correction loop, in which the noise map and the bad pixels maps are updated in each iteration.

I have one remark/question upfront: The noise map is outputted with the bad pixels already masked (as np.nan), as this is a necessary step in the common mode correction loop. If this is problematic, a workaround is to use instead the initial noise map values for these pixels. Evidently, by doing so the noise map values in these pixels will differ significantly from the neighboring pixels, as these are effectively bad pixels, which is why I thought it would make more sense to just keep them masked.

Let me know how I can help you in the review process

I wrote the main comments regarding this MR. So I will give a last look at your response and your updates later.

added 1 commit

• 96cd5476 - delete some plots and other minor revisions

Compare with previous version

added 1 commit

• 30c6516e - restored notebook metadata

Compare with previous version

Summary of changes of the new commit (30c6516e...):

• All the points mentioned in the discussions above
• Removed get_random_db_interface import from cal_tools.tools (was not used)
• Removed import pasha as psh (was not used)
• Added step_timer.done_step() on the read data cell (I had the step_timer.start() on the beginning, but forgot the .done_step())
• Added print(f"Reading from:<file_location.h5>, to print the location and name of the .h5 file as is done in the correction NB, for uniformity. This required doing from pathlib import Path on the second NB cell.
• Changed plot title to CM Corrected Noise Map (added the CM)on histograms and heatmaps for clarity.
• Moved the per ASIC noise histogram plot that was in the last cells of the notebook to the cell where the initial noise map was plotted. This will thus be the only place in the report where the noise map is plotted.
• Reorganised ASIC numbers on based on some ePix documentation. It should be from 0 to 3 starting at the bottom right, and going counter clock wise. This was done on the noise map histogram plot and on the eval_bpidx function
• Changed plot titles from Initial Noise Map to just Noise Map, since this is the only map that will be exported (more on this on the next paragarph). Because the CM corrected noise map is now treated as an auxiliary map to find bad pixels with more precision, there is no need to mention distinguish different noise maps.

An additional important change: The noise map that is injected/saved in this new commit is the initial noise map, and not the CM corrected one. This is because this is the type of map that the Correction NB is expecting, which influences both the CM correction and the charge sharing correction in this notebook. I will investigate if it is advantageous to use the CM corrected noise map and adapting the Correction NB for that, but I will need more time for that. In the meantime is better to keep the type of exported noise map as it is.

added 4 commits

• 2fb5231a - Solve merge conflicts
• 1e76db8c - Solve merge conflicts
• 74a74382 - Solve merge conflicts
• 851020ec - Solve merge conflicts

Compare with previous version

I think these are my last comments.

added 1 commit

• b0b2160b - final minor reviews

Compare with previous version

added 1 commit

• e061bb40 - nbstripout

Compare with previous version

resolved all threads

Here is potentially the last commit. I forgot to do nbstripout on the first commit (b0b2160b), so please ignore that one. Besides the above suggsted changes, I also implemented these:

• Added a comment regarding the vmin/vmax 250 value discussed above
• Passed the offset subtraction to a CM_N_iterations > 0 block, as there is no point in doing offset subtraction without common mode correction.

Thank you @duarten.

LGTM!

changed milestone to %3.8.0

approved this merge request

Hi @duarten,

Please feel free to Merge this MR.

closed

reopened

merged

mentioned in commit 7ebeece7

changed milestone to %3.7.1

removed Waiting for review label