UPDATED: Adding capability to deconvolve using channel-by-channel templates

[vpec0]

This is a reopening of #61.

#61 was merged, caused issues, and reverted in develop.

This PR contains the same commits and additional commits which should resolve the issues in CI tests. FHICL configurations were updated so that they were backwards compatible.

However, FD reco workflows should switch to using table dune_fd_deconvolution from Deconvolution.fcl and ProtoDUNE HD should switch to table protodunehd_deconvolution or protodunehd_mc_deconvolution.

CI tests should be run before the pull, I haven't tested this on FD sims.

[absolution1]

trigger build

[absolution1]

trigger build

Darn it, this did not trigger the CI for me. @aolivier23 how do I trigger the CI within dune's organisation?

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

[FNALbuild]

❌ CI build for DUNE Failed at phase ci_tests DUNE on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests DUNE phase logs

parent CI build details are available through the CI dashboard

[FNALbuild]

❌ CI build for DUNE Failed at phase ci_tests DUNE on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests DUNE phase logs

parent CI build details are available through the CI dashboard

[jroto]

Thanks @vpec0, can you make a PR in dunesw with the changes you mention?

[vpec0]

❌ CI build for DUNE Failed at phase ci_tests DUNE on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests DUNE phase logs

parent CI build details are available through the CI dashboard

There were some configurable name changes which I had not propagated fully. This is fixed in the latest commit (72e0166).

I have tested building the code and run successfully the reco job used in the CI tests:
lar -n 1 -c ci_test_reco_dunefd.fcl -o prodgenie_nue_dune10kt_1x2x6_reco_Current.root -s xroot://fndca1.fnal.gov:1095/pnfs/fnal.gov/usr/dune/persistent/stash/ContinuousIntegration/DUNEFD/detsim/prodgenie_nue_dune10kt_1x2x6_detsim_Reference.root

[vpec0]

Thanks @vpec0, can you make a PR in dunesw with the changes you mention?

@jroto There are these fhicl files which reference dune_deconvolution table:

$ cd dunesw/fcl
$ grep -R dune_deconvolution *
dunefd/reco/workflow_reco_dune10kt.fcl:  opdec:      @local::dune_deconvolution
protodunehd/reco/dqm_reco_protodunehd.fcl:    opdec:              @local::dune_deconvolution
protodunehd/reco/standard_reco_protodunehd_MC.fcl:    opdec:              @local::dune_deconvolution
protodunehd/reco/standard_reco_protodunehd_keepup.fcl:    opdec:              @local::dune_deconvolution
vdcoldbox/reco/crp6_data_reco_daphne_bde2.fcl:       opdec:            @local::dune_deconvolution
vdcoldbox/reco/crp6_data_reco_daphne_bde3.fcl:       opdec:            @local::dune_deconvolution

I assume I can change dunefd/reco/workflow_reco_dune10kt.fcl and the protodunehd fhicls. I don't want to touch vdcolbox files.

How do I make sure that I don't break anything?

[jroto]

I thought @YoannKermaidic got rid of the cold box's deconvolution. As they don't have such a big undershoot, they don't need to deconvolve the signals. Yes, I wouldn't touch those.

If I understood correctly, this deconvolution module has been already tested with data, although not with the new simulation digitizer.
Also, the deconvolution of the full streaming channel is pending. Can you confirm these two points, @vpec0?

Then we need to wait for @mjdelgadog to update the SPE templates of all channels (she told me we would have it by next Monday).

Should we wait for Maritza before merging?

[vpec0]

I can confirm, that the code was not tested with PDHD sims, only with data.

Treatment of full streaming channels is not implemented and those channels should be ignored.

This PR does not need to wait for the templates to be ready. This is functionality implementation and configuration for reading out the templates from stashcache can be added later.

[lpaulucc]

Great!

[lpaulucc]

This looks good, thanks, Viktor.

[jroto]

Can we merge this already? should we run again the CI tests? @absolution1 @aolivier23

[lpaulucc]

trigger build

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

[absolution1]

Looks good, thank you

[FNALbuild]

❌ CI build for DUNE Failed at phase ci_tests DUNE on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests DUNE phase logs

parent CI build details are available through the CI dashboard

[FNALbuild]

⚠️ CI build for DUNE Warning at phase ci_tests DUNE on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests DUNE phase logs

parent CI build details are available through the CI dashboard

[vpec0]

I can't interpret the results from the CI tests. From a quick look, I don't see how the warnings and failures are related to the PR.

[absolution1]

I've just had a look. I think we're due an update of the reference files. I've asked on slack if we can get an update of the refs.