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

Split aqmon hits issue #1061

Closed
wants to merge 19 commits into from
Closed

Split aqmon hits issue #1061

wants to merge 19 commits into from

Conversation

RoBGlaBe
Copy link
Contributor

@RoBGlaBe RoBGlaBe commented Jul 1, 2022

What does the code in this PR do / what does it improve?

Counting veto starts correctly instead of double counting some.

Can you briefly describe how it works?

Veto start signals were counted twice if they started in one data package (for the lack of a better word of "raw_record level chunks") and ended in another due to the usage of hits. Creating hitlets in the same Plugin solves this problem.

Can you give a minimal working example (or illustrate with a figure)?

See note: https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenonnt:robingb:aqmon_hit_splitting

Please include the following if applicable:

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.

Thanks Robin, I think this is exactly what we need. What might help is moving the AqmonDtype class to the AqmonHits plugin - where you can put it in a function infer_dtype-function which is how we usually handle logic like this in strax(en). Then, you can re-use self.dtype and self.dtype.names (to equal the wanted_fields).

@RoBGlaBe
Copy link
Contributor Author

RoBGlaBe commented Jul 5, 2022

Hi @JoranAngevaare,
thank you! I wasn't aware, but definitely saw this function around. I'll change it 👍

# in busy runs veto starts get split at chunk edges into two. we introduce hitlets:
aqmon_hitlets = strax.create_hitlets_from_hits(aqmon_hits, (0,0), [0, 1000])
to_pe = np.ones(808) # stay in ADC units: these are NIM signals
aqmon_hitlets = strax.get_hitlets_data(aqmon_hitlets, records, to_pe=to_pe)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where the current tests are failing. It should actually work like this. When strax.get_hitlets_data is called it lastly complains at https://github.com/AxFoundation/strax/blob/156254287c2037876a7040460b3551d590bf5589/strax/processing/hitlets.py#L214 . My hunch is that we pass something that look's a little different to what https://github.com/AxFoundation/strax/blob/156254287c2037876a7040460b3551d590bf5589/strax/processing/hitlets.py#L222 usually sees and the cached compiled function doesn't like it and seg faults. Any idea on how to fix this? If we delete the cache, we might have the same problem the other way around, no? Maybe we find out what's the difference it doesn't like and provide what it likes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're back to this issue now:

[...]

Fatal Python error: Segmentation fault
Current thread 0x00007fd8cd53f700 (most recent call first):
File "/opt/hostedtoolcache/Python/3.8.13/x64/lib/python3.8/site-packages/strax/processing/hitlets.py", line 214 in get_hitlets_data
File "/home/runner/work///***/plugins/acqmon_processing.py", line 117 in compute
File "/opt/hostedtoolcache/Python/3.8.13/x64/lib/python3.8/site-packages/strax/plugin.py", line 596 in do_compute
File "/opt/hostedtoolcache/Python/3.8.13/x64/lib/python3.8/site-packages/strax/plugin.py", line 488 in iter
File "/opt/hostedtoolcache/Python/3.8.13/x64/lib/python3.8/site-packages/strax/mailbox.py", line 281 in _send_from
File "/opt/hostedtoolcache/Python/3.8.13/x64/lib/python3.8/threading.py", line 870 in run
File "/opt/hostedtoolcache/Python/3.8.13/x64/lib/python3.8/threading.py", line 932 in _bootstrap_inner
File "/opt/hostedtoolcache/Python/3.8.13/x64/lib/python3.8/threading.py", line 890 in _bootstrap

[...]

/home/runner/work/_temp/a65694df-fcc9-4870-821e-663ce8843c36.sh: line 1: 2489 Segmentation fault (core dumped) coverage run --source=*** -m pytest --durations 0
tests/test_daq_reader.py
Error: Process completed with exit code 139.

@RoBGlaBe RoBGlaBe requested a review from JoranAngevaare July 6, 2022 10:04
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 Robin, I cannot see the error you seem to mention - could you have another iteration to have a look?

Comment on lines 308 to 311
@staticmethod
def fake_hit(start, dt=1, length=1):
hit = np.zeros(1, strax.hit_dtype)
aqmon_hit_dtype = strax.unpack_dtype(self.deps['aqmon_hits'].dtype_for('aqmon_hits'))
hit = np.zeros(1, aqmon_hit_dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to add self and remove the static method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, thanks

@JoranAngevaare
Copy link
Contributor

@RoBGlaBe did you manage to fix it?

@JoranAngevaare
Copy link
Contributor

Hi @RoBGlaBe , when you are able to get back to this PR. Could you follow the new plugin structure from #1094? I would consider closing this PR and re-opening a fresh one.

@yuema137 yuema137 closed this Oct 17, 2024
@yuema137 yuema137 deleted the split_aqmon_hits_issue branch October 17, 2024 20:20
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