More ljl_propfunc functionality

[theHenks]

Increase possible functions in the allowed func set for the ljl_propfunc
In particular needed for the SiPM processing and the definition of the LAr anti-coincidence conditions in metadata.

[fhagemann]

One duplicate, otherwise this looks fine to me

[codecov]

Codecov Report

Attention: Patch coverage is 9.80392% with 46 lines in your changes missing coverage. Please review.

Project coverage is 35.42%. Comparing base (c835c1f) to head (9af3a90).

Files with missing lines	Patch %	Lines
src/dataprod_config.jl	0.00%	28 Missing ⚠️
src/utils/pars_utils.jl	0.00%	16 Missing ⚠️
src/legend_data.jl	66.66%	1 Missing ⚠️
src/ljl_expressions.jl	75.00%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (c835c1f) and HEAD (9af3a90). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (c835c1f)	HEAD (9af3a90)
	2	1

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
- Coverage   41.96%   35.42%   -6.54%     
==========================================
  Files          26       25       -1     
  Lines        1959     1863      -96     
==========================================
- Hits          822      660     -162     
- Misses       1137     1203      +66     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[theHenks]

@fhagemann I resolved the duplicates, let's merge.

[oschulz]

@theHenks do you have some examples on how these will be used? I'm worried that we may be pushing this a bit too far, we shouldn't store algorithms (:findnext, :findall, :filter, ...) in metadata strings ...

[theHenks]

@theHenks do you have some examples on how these will be used? I'm worried that we may be pushing this a bit too far, we shouldn't store algorithms (:findnext, :findall, :filter, ...) in metadata strings ...

I added these since I want to have variety in the description of the LAr anti-coincidence classifier and to express this in metadata. So far, we do, for example something like this hard-coded in LegendDataManagement:

let a = a, m = m
        @pf (
            trig_pe = $trig_max .* m .+ a,
            trig_is_dc = [any(abs.($trig_pos_DC .- pos) .< 100u"ns") for pos in $trig_pos],
        )
 end

Since the discharge DC removal might require a more difficult software trigger layout, I wanna be flexible in the description of such cut (and have the option to set different ones in the legend-metadata e.g.

 trig_is_dc = [any(abs.($trig_pos_DC[findall.($trig_max_DC .> trig_max_DC_threshold)] .- pos) .< 100u"ns") for pos in $trig_pos],

Etc.
The reduce, vcat and hcat functions are required since typically all SiPM analysis outputs are VectorsOfVectors and you need to flatten them after some initial cuts.
I am open to any other suggestions, @oschulz , but I want to avoid changing a LAr cut or PE reconstruction by opening a PR here every time we come up with a different scheme. Also, this will be even harder in my opinion if we start having different schemes for different ChannelIds.

[oschulz]

We should not turn metadata into a generic programming language, that's a rabbithole. Values and simple value combinations ("calibration functions") yes (like the 100u"ns"), but not algoritms, even small ones. We can have several algorithms defined in the code and let metadata select which one to apply, but putting algorithmic constructs into metadata with quickly become unmaintainable and also effectively means a huge (not so well) hidden code duplication.

[theHenks]

I see your point. But then do you propose to put e.g. all the LAr cut generation into separate functions?
I just find this hard to develop and iterate on because each change will require a PR and massively slow down the development. In addition, how do you want to handle different functions for different channels? Each channel then gets its own function?

[oschulz]

In addition, how do you want to handle different functions for different channels? Each channel then gets its own function?

Are we really going to use a large number of different algorithms for different channels?

[fhagemann]

How about this: in order to meet the unblinding deadline, we merge this PR as is.
@theHenks provides examples of how it is used and, in the meantime, @oschulz can develop an alternative implementation that will at some point become the successor of this and make us revert this?

[fhagemann]

I force-pushed to resolve the merge conflict introduced after merging #79

[oschulz]

in order to meet the unblinding deadline

Will we use channel-dependent code for the unblinding? Also, the code that generates the files will need to know the functional form(s) - and so that's already under version control. So we may as well have the algorithm function under version control directly. JSON strings should not be used to exchange code between packages.

how do you want to handle different functions for different channels?

I assume we'll have a finite number of them, though? In the end, this is similar to DSP, we also don't put DSP chains as code into JSON files.

[theHenks]

Yes, we will probably have to handle channels independently because of the cross-talk between SiPMs and HPGe channel.

Can you tell us which functions you don't want to have in here? If I can't put this in here than I have to move the whole logic into LegendEventAnalysis. And have a lot of subcases.

[oschulz]

Can you tell us which functions you don't want to have in here?

Definitely not :findfirst, :findnext, :findall, :filter, :map, :reduce, :vcat, :hcat, :hvcat, :cat, :reshape, :transpose, :permutedims, :sort, :sortperm .

I'm also very doubtful about :getproperty, :getindex, :keys, :values, :pairs, :haskey .

[theHenks]

:getproperty, :getindex etc. are already used for the QC cuts. E.g. here under "qc":.

Same for :reduce and :vcat. I could live with removing these: :findfirst, :findnext, :findall, :filter, :map, :reduce, :vcat, :hcat, :hvcat, :cat, :reshape, :transpose, :permutedims, :sort, :sortperm
However, I am not 100% sure if one is already used somewhere since I can't check the whole data flow. So, I suggest temporarily removing those and adding one or so in a separate PR in case something breaks until it is moved.

[oschulz]

Oh, sorry, yes, we already had :getproperty - we'll keep it for compatibility, of course, but that use case we can solve properly (support a.c.b and so on): #83

[oschulz]

As far as :getindex is concerned, sorry, I forgot that we didn't have support for a[i] yet, #83 takes care of that as well, allowing for expressions like a.b[2].

[oschulz]

What's an example use case for reduce and vcat?

[oschulz]

In general I'd like ljl-expressions to be constant-time (no searches, iterations, etc.).

Actually we may want/need sum and prod.

[oschulz]

Some of the requested functions (not :keys, :values, :pairs, :findfirst, :findnext, :findall, :filter, :map, :reduce, :vcat, :hcat, :hvcat, :cat, :reshape, :transpose, :permutedims, :sort, :sortperm) have been added in #83 now.

[oschulz]

The PR now includes a lot of reversals to old code?

[theHenks]

I am sorry, I messed up my git. Will fix that!