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

Incorrect time window for the charge extraction in DQM #43

Closed
jlenain opened this issue Feb 14, 2023 · 2 comments · Fixed by #45
Closed

Incorrect time window for the charge extraction in DQM #43

jlenain opened this issue Feb 14, 2023 · 2 comments · Fixed by #45
Labels
bug Something isn't working

Comments

@jlenain
Copy link
Collaborator

jlenain commented Feb 14, 2023

While reviewing #42 , I saw a potential bug in the configuration of LocalPeakWindowSum used for the charge extraction in the Data Quality Monitoring module. The configuration is as follows:

        config = Config(
            {"LocalPeakWindowSum": {"window_shift": 12, "window_width": 12}}

The charge is then extracted over 12 ns, but with a time reference 12 ns before the actual pulse peak.

Could you please have a look, @hashkar ?

@jlenain jlenain added the bug Something isn't working label Feb 14, 2023
@hashkar
Copy link
Collaborator

hashkar commented Feb 20, 2023

Yes @jlenain the values here are not right, they are the leftover of a very old example. As I don't know what the exact values should be we can either leave it for the user to choose. If we now know what the values should be we can fix it once and for all. Otherwise we can discuss it during the software meeting.

@jlenain
Copy link
Collaborator Author

jlenain commented Feb 20, 2023

Hi @hashkar !
Thanks for the reply.
A mechanism could indeed be introduced to let the user choose the time window to use.
However, in the meantime I would also propose to fix the current window to better encompass the pulse signal, e.g. [-4;+12] ns.

jlenain added a commit to jlenain/nectarchain that referenced this issue Feb 20, 2023
@jlenain jlenain linked a pull request Feb 20, 2023 that will close this issue
dkerszberg added a commit that referenced this issue Feb 20, 2023
jlenain added a commit to jlenain/nectarchain that referenced this issue Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants