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

refresh nticks for each event #317

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wenqiang-gu
Copy link
Member

@wenqiang-gu wenqiang-gu commented Jul 3, 2024

In ProtoDUNE HD, the lengths of raw::RawDigits are not fixed among different events. Currently, the OmnibusNoiseFilter will determine "nticks" at the beginning of configuration and will not update it later. The purpose of this PR is to refresh "nticks" for each event.

To use it, just add reload_nticks: true in the nf.jsonnet (the default value is false).

   local obnf = g.pnode({
        type: 'OmnibusNoiseFilter',
        name: name,
        data: {

            // Nonzero forces the number of ticks in the waveform
            nticks: 0,
            reload_nticks: true,


@brettviren
Copy link
Member

Locally, this looks reasonable to me, @wenqiang-gu . In hindsight, we should have started out with this assumption of variable length signals...

It seems to me that the reload_nticks (as a configuration parameter) is redundant with the configuration nticks=0. Of course we still need an internal bool to flag the operational mode. If the configuration is truly redundant, it is best to remove reload_nticks (internally test on initial nticks==0) as the result will be more robust against user config errors.

I do have a concern for possible side-effects in the actual noise filters.

First, are there any noise filters which are configured with their own fixed nticks value and if so, how do they react to input signals with differing sizes?

Second, the harmonic filter(s) at some point (still true?) were configured based on frequency bins and not frequency ranges. Specifying with bins locks them into only working properly when given their expected, fixed size signal. They should be modified to be configured with frequency ranges and to recalculate the bins to mask anytime an input nticks changed. For backwards compatibility, they could still accept (optional/depreciated) config forms which are in the current terms of bins and nticks but convert that and store that info as frequency range.

Please check if there are any cases like this where the user will get a surprise. If it happens that PDHD does not use any of these problematic filters, then at least please make a new ticket that lists the work needed to fix these problematic filters.

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.

2 participants