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

Allow set number of modules #183

Closed

Conversation

pawel21
Copy link
Collaborator

@pawel21 pawel21 commented Oct 3, 2019

Issue #182

I have modified code to allow set number of modules as parameter.
This will allow use low level corrections, when modules as missing (works least than 265 modules)
The parameter n_module must agree with number of module use to create pedestal file.

@rlopezcoto
Copy link
Contributor

Hi @pawel21, I see a potential problem here. When we perform regular observations, the file used to produce a pedestal will likely have the same number of modules as the one used for data. As we are working right now, we may use a file to calculate the pedestal that has all the modules, but the data one has less modules.
For me, ideally, if the file for the pedestal covers all the modules and the one for the data has less modules, the correction should still work. If the file used for the pedestal has some missing modules that are present in the data file, then an error should be issued. Would it be possible to program it that way?

@FrancaCassol
Copy link
Collaborator

Dear @pawel21,
if I understand correctly what you are doing, you don't need any input parameters. As said in issue #182, you must specifically use the module numbers contained in the event.lst.tel[tel_id].svc.module_ids array when you perform the loop on the modules. In this array you have the number of modules present during the run (for example at present the 118 is missing).
Obviously, the pedestal run must have at least all the modules of the data-taking runs, that means if during the data-taking a module is switched on, the shifters must perform a new pedestal run.

@pawel21
Copy link
Collaborator Author

pawel21 commented Oct 3, 2019

Generally, my code to perform correction using number of pixels from current event n_modules = event.lst.tel[self.tel_id].svc.num_modules.

@FrancaCassol Thank you for explanation. You are right.
I will change it.
I will try do as fast as I can, but I am at MAGIC shift, so I am a bit busy.

I see another problem in function

def _load_calib(self):
        """
        Function to load pedestal file.
        """
        if self.pedestal_path:
            with fits.open(self.pedestal_path) as f:
                pedestal_data = np.int16(f[1].data)
                self.pedestal_value_array[:, :, :self.size4drs] = pedestal_data - self.offset
                self.pedestal_value_array[:, :, self.size4drs:self.size4drs + 40] \
                    = pedestal_data[:, :, 0:40] - self.offset

When the shape of pedestal_data from pedestal file not agree with shape of pedestal_value_array (array store pedestal value, which is created in __init__ [line 114] )
When we use pedestal file, which covers all pixels, correction should work well.

@pawel21
Copy link
Collaborator Author

pawel21 commented Oct 4, 2019

@FrancaCassol I am using expected_pixel_id = ev.lst.tel[tel_id].svc.pixel_ids to obtain pixel ids to perform correction and I think this field contain only "good" pixels (without pixel from "dead" module).
What do you think about it?

@pawel21
Copy link
Collaborator Author

pawel21 commented Oct 4, 2019

I found bug in code to create pedestal file, when not all the modules are on.
I will fix it.

@pawel21
Copy link
Collaborator Author

pawel21 commented Nov 18, 2019

I closed.
I think, we don't need set number of modules as parameter.
About pedestal file: I have another PR #209 , where I solved problem with missing module.

@pawel21 pawel21 closed this Nov 18, 2019
@pawel21 pawel21 deleted the allow_set_number_of_modules branch November 18, 2019 16:06
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.

3 participants