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

Acqmon Deadtime Plugin #207

Merged
merged 13 commits into from Sep 29, 2020
Merged

Acqmon Deadtime Plugin #207

merged 13 commits into from Sep 29, 2020

Conversation

AlexElykov
Copy link
Contributor

@AlexElykov AlexElykov commented Sep 3, 2020

What is the problem / what does the code in this PR do
Calculating deadtimes for busy/veto monitor such that we can use them later for NODIAQ busy monitoring

Can you briefly describe how it works?
Looks for start and end times of veto signals and finds the intervals in between them

Can you give a minimal working example (or illustrate with a figure)?
Seems to find the correct deadtime intervals.
sample_pic

Additional info: I have another plugin that uses the new "veto_intervals" to calculate cumulative deadtimes and put it into a collection on mongo. However, we need to decide in which collection it should go and which fields we want. @darrylmasson

Test notebook can be found here: https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenon1t:alexelykov:am_veto_interval_notebook

@AlexElykov
Copy link
Contributor Author

CodeFator issue just fixed by adding few more lines at the end the acqmon_processing here: c1b9731

Copy link
Contributor

@darrylmasson darrylmasson left a comment

Choose a reason for hiding this comment

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

I don't know if the dtype works

straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
@AlexElykov
Copy link
Contributor Author

AlexElykov commented Sep 11, 2020

I've made some modifications to the acqmon plugin.

  • Added a small plugin (producing acqmon_hits) which should allow more flexibility for looking into non veto channels of the AM such as the GPS sync and maybe sum_wf
  • Addressed the previously mentioned issues
  • Changed the veto_intervals data kind to a more streamlined version based on Darryl's suggestion

Example of a result from this plugin.

array([(1595524677117194400, 1595524677330501560, 213307160, 'busy_veto'),
(1595524678056589480, 1595524678154909520, 98320040, 'busy_veto'),
(1595524678998968760, 1595524679097664400, 98695640, 'busy_veto'),
(1595524679941977400, 1595524680082552780, 140575380, 'busy_veto'),
(1595524683714345290, 1595524683876195570, 161850280, 'busy_veto'),
(1595524686544301810, 1595524686707251480, 162949670, 'busy_veto')
dtype=[(('veto start time since unix epoch [ns]', 'time'), '<i8'),
(('veto end time since unix epoch [ns]', 'endtime'), '<i8'),
(('veto interval [ns]', 'veto_interval'), '<i8'), (('veto signal type', 'veto_type'), '<U10')])

Copy link
Contributor

@JoranAngevaare JoranAngevaare left a comment

Choose a reason for hiding this comment

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

Hi Alex. Hereby some requests for changes.

I've looked at your PR and it generally looks good. Also the tests and output look intuitive https://github.com/XENONnT/analysiscode/blob/master/StraxTests/AqMon.ipynb.

I'll fix the failing travis in a bit. That is actually my fault rather than yours.

straxen/plugins/acqmon_processing.py Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/contexts.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
@AlexElykov
Copy link
Contributor Author

Hi, @WenzDaniel can you please review the recent updates and approve the PR. I think I've addressed all the requests/concerns that were raised thus far.

Copy link
Collaborator

@WenzDaniel WenzDaniel left a comment

Choose a reason for hiding this comment

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

Hey Alex, I am sorry, but in case I am not still half asleep due to XXXX, I think your code is not working as you would expect. Could you please address my comments and ensure with your example notebook that it is really working as intended?

straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
@AlexElykov AlexElykov added the enhancement New feature or request label Sep 23, 2020
@AlexElykov
Copy link
Contributor Author

@jorana @WenzDaniel
Made additional fixes to the aqmon based on your input:

  • Added the deault start channel for hardware aqmon in common.py
  • Fixed a bug in the resulting dict when enries were overwritten by new ones.

Tested with few runs, veto intervals seem to be correct. Simple test notebok can be found here:
https://gist.github.com/AlexElykov/b804b03f1951dfe534a56c3b51487059

Copy link
Collaborator

@WenzDaniel WenzDaniel left a comment

Choose a reason for hiding this comment

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

Only two minor comments. The rest looks fine. Thanks a lot.

straxen/plugins/acqmon_processing.py Outdated Show resolved Hide resolved
straxen/plugins/acqmon_processing.py Show resolved Hide resolved
@AlexElykov
Copy link
Contributor Author

Alright, more updates based on the input, with the 2 main ones being:

  • Removed the return from the setup function
  • Result from plugin is sorted by "time"

Plugin was tested with the same notebook as before, seems to work.

res_temp.setdefault("veto_interval",[]).extend((vetos['endtime'] - vetos['time']))
res_temp.setdefault("veto_type",[]).extend([veto + 'veto']*len(vetos))

res = strax.dict_to_rec(res_temp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

There seems to be an error with empty dicts:

x = {}, dtype = None

    @export

    def dict_to_rec(x, dtype=None):

        """Convert dictionary {field_name: array} to record array

        Optionally, provide dtype

        """

        if isinstance(x, np.ndarray):

            return x

    

        if dtype is None:

            if not len(x):

>               raise ValueError("Cannot infer dtype from empty dict")

E               ValueError: Cannot infer dtype from empty dict

Copy link
Contributor

Choose a reason for hiding this comment

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

res = strax.dict_to_rec(res_temp, dtype = self.dtype)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, sorry forgot to copy the line

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, yeah it's the trivial solution to fix Travis. Alex I just added it in d92de8c. I believe the review has been quite thorough so let's merge it! Would love to have this shown on the website.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WenzDaniel @jorana Cheers guys :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants