Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve create pedestal script #209

Merged

Conversation

pawel21
Copy link
Collaborator

@pawel21 pawel21 commented Nov 9, 2019

Hi,
I have made few improvement in code to create pedestal file

  1. Add optional argument to set start sample for waveform, which is using to create pedestal table. Default value is 11.
    The 2 first samples are more noise and the first 9 samples have occasionally increased signal due to Tsutomu pattern, hence we should skip first 11 samples.
    Average signal should be around zero after the low level corrections with new pedestal file (so far is ~ 5/10 below 0).

  2. Set constant value (1855) of number of pixels n_pixels to initialize pedestal array. (This solve problem when some modules are missing)

@FrancaCassol
Copy link
Collaborator

Hi @pawel21,

self.n_pixels = 265*n_channel

could you also assign the 265 to a constant variable on top of the module?

I have a question, if one module is missing is the first_capacitor array shorter? (I could check but right now I don't remember which run was with a missing module)

@pawel21
Copy link
Collaborator Author

pawel21 commented Nov 12, 2019

Hi @FrancaCassol ,

could you also assign the 265 to a constant variable on top of the module?

Yes, I will do it.

I have a question, if one module is missing is the first_capacitor array shorter? (I could check but right now I don't remember which run was with a missing module)

Yes, if one (or more) module is missing, the first_capacitor_array is shorter (this allow iterating over working modules)
Run 1408 is with missing module

@pawel21
Copy link
Collaborator Author

pawel21 commented Nov 19, 2019

Dear @rlopezcoto, @FrancaCassol
Could you look on this PR?

@@ -13,23 +16,32 @@
roi_size = 40
high_gain = 0
low_gain = 1
N_module_in_camera = 265
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now this number is hardcoded, if we have observations with less modules how can you change it?
For the naming:
N_module_in_camera -> n_modules_in_camera

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable is used to create array with pedestal values. (I need it to avoid run out of range by numba function, when module is missing) To iterate over module code uses self.n_module read from data file.
Naming: I have changed.

scripts/lstchain_data_create_pedestal_file.py Outdated Show resolved Hide resolved
scripts/lstchain_data_create_pedestal_file.py Outdated Show resolved Hide resolved
@pawel21
Copy link
Collaborator Author

pawel21 commented Nov 19, 2019

I have changed default ,,maximum numbers of events to read" from 5000 to 8000

@FrancaCassol
Copy link
Collaborator

This code seems ok, could we merge it?

@rlopezcoto rlopezcoto merged commit e909cb6 into cta-observatory:master Nov 20, 2019
@pawel21 pawel21 deleted the improve_create_pedestal_script branch July 10, 2020 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants