Time lapse corr spike b #167

pawel21 · 2019-09-09T11:45:12Z

I have modified code according Seiya's Nozaki idea:

Updating times from slices -1 to 38 remove spike b (small jump of signal) without interpolation (Seiya Nozaki idea).
Interpolation spike A is only needed for even last capacitor (lc) in the first half of the DRS4 ring.

rlopezcoto

I would add some comments on the code about the nature of the spikes and why they need to be interpolated
It would also be great to have some pep8-ing of the code (spaces between operator...) to improve the readability.

rlopezcoto · 2019-09-23T09:59:51Z

lstchain/calib/camera/r0.py

@@ -341,15 +340,18 @@ def do_time_lapse_corr(waveform, expected_pixel_id, local_clock_list, fc, last_t
                            val = waveform[gain, pixel, k] - ped_time(time_diff_ms)
                            waveform[gain, pixel, k] = val

+                # updating times from slices -1 to 38 remove spike b (small jump of signal)
+                # without interpolation (Seiya Nozaki idea)
                posads0 = int((0 + fc[nr_module, gain, pix]) % size4drs)
                if posads0+40 < 4096:


instead of 4096, I would use some variable name, like len_drs4

rlopezcoto · 2019-09-23T10:02:48Z

lstchain/calib/camera/r0.py

-                        interpolate_spike_B(waveform, gain, spike_b_position, pixel)
+                            # The correction is only needed for even last capacitor (lc) in the
+                            # first half of the DRS4 ring
+                            if (fc_old[nr_module, gain, pix] + 39) % 2 == 0 and (fc_old[nr_module, gain, pix] + 39) % 1024 <= 510:


numbers like 38, 39 could be len_drs4_window (or something like that)

rlopezcoto · 2019-09-23T10:10:28Z

lstchain/calib/camera/r0.py

                else:
-                    for k in prange(0, 39):
+                    for k in prange(-1, 39):
                        posads = int((k + fc[nr_module, gain, pix]) % size4drs)


shouldn't this run to 38?

pawel21 · 2019-10-03T05:00:30Z

@rlopezcoto Thanks for comments.
Now, I am at MAGIC shift, so I am a bit busy and need some time to improve code.

FrancaCassol · 2019-11-08T08:32:15Z

Hi @pawel21,
is this PR valid only before run 1575?
I wonder, if the change of firmware implies only the change of the sample range (first and last slice to be considered), should we not simply define it using variables that are assigned as trailets so to keep only one corrector class for all the runs? (the main code will verify which value to use as function of the run). Or there are other changes necessary for the new firmware?

jsitarek · 2019-11-08T13:40:08Z

Hi @FrancaCassol
In the script to calculate pedestal coefficients we plan to do exactly what you are saying, there is is very simple and indeed it is better not to duplicate the functions.

But the dt correction function and spike interpolation are messy, because multiple conditions change, and also things can be optimized differently (e.g. the old correction required updating times from one capacitor before the actual readout to 38th capacitor including, and the code was optimized to avoid too many loops. Now the special cases when you cross the ring look different.

pawel21 · 2019-11-15T15:11:17Z

Hi @rlopezcoto , @FrancaCassol , @SeiyaNozaki
I have updated code according to new DRAGON firmware, and I have created function with "old" code to allow using low level calibration on "old" data before 2019/11/05 (new firmware update date).
We (me and Julian) tested code on "old" data (Run 1532) and "new" data (Run 1579), seems work OK. Code itself check run id, then according run id choosing function to low level calibration.

I made some clean-up.
I know that the code still needs cleaning, but now before observing Crab I think that the most important thing is to have working code (for new firmware) in the main repository.

Please look at my new codes and get this into the lstchain master as soon as possible.

rlopezcoto

Thanks @pawel21, this looks very good, I'd recommend it to be merged as soon as you have performed the small corrections I suggested

rlopezcoto · 2019-11-17T17:01:04Z

lstchain/calib/camera/r0.py

@@ -252,24 +315,80 @@ def interpolate_pseudo_pulses(waveform, expected_pixel_id, fc, fc_old, n_modules
                for pix in prange(0, n_pix):
                    for k in prange(0, 4):
                        # looking for spike A first case
-                        abspos = int(1024 - roisize - 2 - fc_old[nr_module, gain, pix] + k * 1024 + size4drs)
+                        abspos = int(1025 - roi_size - 2 - fc_old[nr_module, gain, pix] + k * 1024 + size4drs)


Can we define 1025 as size1drs + 1, with size1drs = 1024 or something like that?
Could you please also add a small explanation about this change as a comment? we had some e-mail interchange that may be lost for the future...

rlopezcoto · 2019-11-17T17:01:50Z

lstchain/calib/camera/r0.py

+                            # last capacitor (lc) in the first half of the
+                            # DRS4 ring
+                            if ((fc_old[nr_module, gain, pix] + (roi_size-1)) % 2 == 0 and (fc_old[nr_module, gain, pix] + (roi_size-1)) % 1024 <= 511):
+                                pixel = expected_pixel_id[nr_module * 7 + pix]


same here, 511 can be size1drs/2 - 1

rlopezcoto · 2019-11-17T17:02:14Z

lstchain/calib/camera/r0.py

+                for pix in prange(0, n_pix):
+                    for k in prange(0, 4):
+                        # looking for spike A first case
+                        abspos = int(1024 - roi_size - 2 -fc_old[nr_module, gain, pix]+ k * 1024 + size4drs)


same comments as above

rlopezcoto · 2019-11-17T17:03:53Z

lstchain/calib/camera/r0.py

+    roi_size = 40
+    for nr_module in prange(0, number_of_modules):
+        time_now = local_clock_list[nr_module]
+        for gain in prange(0, 2):


(0, n_gain)

rlopezcoto · 2019-11-17T17:04:17Z

lstchain/calib/camera/r0.py

+    for nr_module in prange(0, number_of_modules):
+        time_now = local_clock_list[nr_module]
+        for gain in prange(0, 2):
+            for pix in prange(0, 7):


FrancaCassol · 2019-11-18T11:23:08Z

lstchain/calib/camera/r0.py

@@ -170,25 +191,57 @@ def time_lapse_corr(self, event, tel_id):
        event : `ctapipe` event-container
        tel_id : id of the telescope
        """
+
+        run_id = event.lst.tel[1].svc.configuration_id


Hi @pawel21,
please, change tel[1] with tel[tel_id]

FrancaCassol · 2019-11-18T12:43:30Z

Hi @rlopezcoto ,

for me this code is good for merging

pawel21 added 5 commits August 25, 2019 12:02

code motification to interpolate spike A and B

44d3070

remove interpolation spike b and add comments

baafafd

clean up code

6fb045f

clean up code

03a6e5a

delta t update time fix

b101ef6

rlopezcoto reviewed Sep 23, 2019

View reviewed changes

some clean up

be6cd9e

pawel21 added 8 commits November 12, 2019 10:10

update branch from master

851d629

Change code according new firware

f949dd8

update time_lapse_corr function

79ff518

created new and old version of interpolate spikes

a931691

improved condition id dt corr

db3088a

cleaned up code

1f18ae7

refactoring code, add roi_size

ff40253

add old and new function to dt on R0

bbaf4da

rlopezcoto reviewed Nov 17, 2019

View reviewed changes

pawel21 added 2 commits November 18, 2019 10:39

Changed code according Ruben comment

0faaed7

add comments

d4aacdc

FrancaCassol reviewed Nov 18, 2019

View reviewed changes

fixed bug with tel_id

ae062b7

rlopezcoto merged commit 8bdd812 into cta-observatory:master Nov 18, 2019

pawel21 deleted the time_lapse_corr_spike_b branch November 18, 2019 15:55

rlopezcoto mentioned this pull request Dec 12, 2019

AttributeError: 'NullR0Calibrator' object has no attribute 'tel_id' #207

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Time lapse corr spike b #167

Time lapse corr spike b #167

pawel21 commented Sep 9, 2019

rlopezcoto left a comment

rlopezcoto Sep 23, 2019

pawel21 Nov 15, 2019

rlopezcoto Sep 23, 2019

pawel21 Nov 15, 2019

rlopezcoto Sep 23, 2019

pawel21 commented Oct 3, 2019

FrancaCassol commented Nov 8, 2019

jsitarek commented Nov 8, 2019

pawel21 commented Nov 15, 2019

rlopezcoto left a comment

rlopezcoto Nov 17, 2019

pawel21 Nov 18, 2019

rlopezcoto Nov 17, 2019

pawel21 Nov 18, 2019

pawel21 Nov 18, 2019

rlopezcoto Nov 17, 2019

pawel21 Nov 18, 2019

rlopezcoto Nov 17, 2019

pawel21 Nov 18, 2019

rlopezcoto Nov 17, 2019

pawel21 Nov 18, 2019

FrancaCassol Nov 18, 2019 •

edited

Loading

pawel21 Nov 18, 2019

FrancaCassol commented Nov 18, 2019

Time lapse corr spike b #167

Time lapse corr spike b #167

Conversation

pawel21 commented Sep 9, 2019

rlopezcoto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawel21 commented Oct 3, 2019

FrancaCassol commented Nov 8, 2019

jsitarek commented Nov 8, 2019

pawel21 commented Nov 15, 2019

rlopezcoto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrancaCassol Nov 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrancaCassol commented Nov 18, 2019

FrancaCassol Nov 18, 2019 •

edited

Loading