[JUNGFRAU] [Dark] Fix bad pixel constant from darks taken in burst mode

Description

Corrected JUNGFRAU data in burst mode (16 cells) was getting a mask which was as expected for the first memory cell used, but in the other 15 cells, every pixel was marked with WRONG_GAIN_VALUE. The mask is used for the ROI reduction, which was therefore giving nan.

The issue is in the BadPixelsDark10Hz constants. For medium & low gains in burst mode, all but the first memory cell are marked as invalid. Then a check for the gain stage of each pixel gets nan, because there's no data, so it thinks all pixels are in the wrong gain stage. Finally, the code added in !914 (merged) 'reflects' WRONG_GAIN_VALUE into the high gain stage, where we do have valid data.

The solution here is to skip the processing entirely when no data is found for a memory cell. Where this is the case, I set the bad pixel constant to NO_DARK_DATA. It also reports which cells are missing.

How Has This Been Tested?

Run the dark notebook interactively, working with p6616 runs 272, 273 & 274 (JNGFR02) and checked the resulting constants against the existing ones from this report.

• The offset constant is the same in high gain (offset[..., 0]) and the same for the first memory cell used (offset[:, :, 15]).
• For the other cells in medium & low gain (offset[:, :, :15, 1:]), the offset changes from nan to 0. I can put it back to nan if we prefer. changed to preserve the nan values.
• The cells in medium & low gain for all but the first memory cell used (badpx[:, :, :15, 1:]) go from bad pixels WRONG_GAIN_VALUE | OFFSET_NOISE_EVAL_ERROR to now NO_DARK_DATA | NOISE_OUT_OF_THRESHOLD | OFFSET_OUT_OF_THRESHOLD, because of 0s replacing NaNs. NO_DARK_DATA | OFFSET_NOISE_EVAL_ERROR.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• My code follows the code style of this project.

Reviewers

@ahmedk @schmidtp

added 1 commit

• cda58679 - Set offset & noise constants to NaN with no data

Compare with previous version

changed the description

I decided that putting NaN in the offset & noise constants was the right thing to do: it's not an estimate that we think might be bad, there's actually no estimate at all for these values. The latest commit does that.

This means that the offset constant is now completely unchanged from before; only the bad pixel map changes.

added 1 commit

• a845950d - Fix spelling of variable name in CI config

Compare with previous version

mentioned in merge request !933 (merged)

what is the reason behind the failing FXE_XAD_JF1M test? Were there 255 cells as well for the reference run we used?

In burst mode with dynamic gain, we set cells to 255 in acelltable. I assume the dark runs we use in the test were taken with dynamic gain.

        # Only for dynamic medium and low gain runs [forceswitchg1, forceswitchg2] in burst mode.

        if gain_mode == 0 and gain > 0 and memory_cells == 16:
            # 255 similar to the receiver which uses the 255
            # value to indicate a cell without an image.
            # image shape for forceswitchg1 and forceswitchg2 = (1024, 512, 2, trains)
            # compared to expected shape of (1024, 512, 16, trains) for high gain run.
            acelltable[1:] = 255

Thank you. Sorry, I was confused with the summary plots for the badpixels in medium and low gain.

That is kind of confusing, but I think it's what I'd expect. The first plot is an average over all memory cells, and there are bad pixels for 15 of 16 cells. The colour scale is fixed to 0-5 in the summary notebook.

The other two show a numeric change from the previous constant, because we're going from a large number for WRONG_GAIN_VALUE | OFFSET_NOISE_EVAL_ERROR to a much smaller one for NO_DARK_DATA | OFFSET_NOISE_EVAL_ERROR.

I wonder if we can find a better way to visualise the bad pixel enums.

I have contacted Marco to update him on this. I understand FXE was contacted before to not take dynamic dark data for Jungfrau.

LGTM.

I wonder if we can find a better way to visualise the bad pixel enums.

I tried to find something useful before but didn't reach a working plot. I think I might have another try with the calephant work soon

BTW these plots at least works very good after your changes. Also for the summary of each map

changed milestone to %3.12.0

I don't have more comments on the MR but I would like to confirm these changes with Marco

added Test CAL Data Affected label

Last question: is it known why there are cells missing dark data for the latest runs (273, 274 proposal 6616) for FXE?

I don't think there actually are cells missing dark data, but the code snippet I posted above sets acelltable to 255 (i.e. missing) for most of the memory cells in med/low gain. So it's the dark processing that decides this data is 'missing'.

So this part is very very confusing for me. After many discussions with macro I understand now what is going on.

1. The runs (273, 274 proposal 6616) and the FXE JF1M burst mode dark runs we use for our tests are runs not taken by the dark middle layer device.
2. This condition

        # Only for dynamic medium and low gain runs [forceswitchg1, forceswitchg2] in burst mode.

        if gain_mode == 0 and gain > 0 and memory_cells == 16:
            # 255 similar to the receiver which uses the 255
            # value to indicate a cell without an image.
            # image shape for forceswitchg1 and forceswitchg2 = (1024, 512, 2, trains)
            # compared to expected shape of (1024, 512, 16, trains) for high gain run.
            acelltable[1:] = 255

should only be done for dark runs acquired with the middle layer device.

As can be seen from the shapes in the comment the forceswitchg1 and forceswitchg2 are expected to have (1024, 512, 2, trains) not (1024, 512, 16, trains). (1024, 512, 16, trains) is the shape for the runs I mentioned in the first point.

In summary, this condition should be fixed to check the index of the storage cells before setting any values to 255.

Below are 2 reports. First is the current implementation(including this MR) and the second report is the exclusion of the pointed at condition

255_assignment.pdf

no_255_assignment.pdf

The summary plots make more sense now.

I will try to find a burst mode dark runs taken with the Middle layer device

I don't think there actually are cells missing dark data, but the code snippet I posted above sets acelltable to 255 (i.e. missing) for most of the memory cells in med/low gain. So it's the dark processing that decides this data is 'missing'.

Thank you for clarifying this. This should not be the case.

Aha, thanks, that makes sense. So should we add a check there like images.shape[2] == 2? Do you prefer to include this in this MR, or should we merge this one and fix that separately?

I think we should fix it separately yes.

Great, in that case I think I can merge this.

merged

mentioned in commit baa11f54