added EventNtuples to CRV Pass1 script

[ehrlich-uva]

No description provided.

[oksuzian]

Let me review this pull request in detail.Here's my review of PR #7 — "added EventNtuples to CRV Pass1 script" by @ehrlich-uva.

Summary

This PR makes 5 changes across the CRV FHiCL configuration files:

1. Removes the useSubsystem0 parameter from CrvCalibration_extracted.fcl and CrvPedestal_extracted.fcl
2. Modifies CrvPass1_extracted.fcl — removes the NZS output module and renames the TFileService output
3. Adds CrvPass1_extracted_combined.fcl — a new combined script that runs CRV reco + EventNtuples in one job
4. Adds CrvPass1_extracted_eventntuples.fcl — a standalone script that runs only EventNtuples on already-reconstructed data

---

Issues Found

1. 🔴 process_name collision between _combined.fcl and original CrvPass1_extracted.fcl

process_name : CrvPass1

The new CrvPass1_extracted_combined.fcl uses process_name : CrvPass1, which is the same process name as the original CrvPass1_extracted.fcl. If the combined script is run on output from the original Pass1 (or vice versa), art will refuse to process the file because it detects a duplicate process name. Consider renaming it to something like CrvPass1Combined.

2. 🟡 TFileService.fileName conflict / ambiguity

services.TFileService.fileName : "crvRecoExtracted.root"

services.TFileService.fileName : "crvRecoExtracted_DQM.root"

The combined file uses "crvRecoExtracted.root" for its TFileService output — the same name the original CrvPass1_extracted.fcl used before this PR renamed it to "crvRecoExtracted_DQM.root". This means if both scripts are run in the same working directory (or in a workflow that doesn't carefully separate outputs), the ROOT files will silently overwrite each other. Consider giving _combined.fcl a distinct name like "crvRecoExtracted_combined.root".

3. 🟡 Missing ProditionsService / DbService config in CrvPass1_extracted_eventntuples.fcl

services.TFileService.fileName : "crvRecoExtracted_EventNtuples.root"
services.GeometryService.inputFile: "Offline/Mu2eG4/geom/geom_common_extracted.txt"
services.TimeTracker.printSummary: true
services.scheduler.wantSummary: true
services.message.destinations.log.outputStatistics : true

The standalone EventNtuples script is missing these service configurations that are present in both CrvPass1_extracted.fcl and CrvPass1_extracted_combined.fcl:

services.ProditionsService.crvStatus.useDb: true
services.ProditionsService.crvCalib.useDb: true
services.DbService.textFile : [...]
services.DbService.verbose : 0

If the EventNtuple analyzer reads CRV digi or reco pulse information (which it does — FillCRVDigis : true), it may need the Proditions/DB calibration and status tables to be properly configured. This could cause runtime errors or incorrect results.

4. 🟡 CrvDigisTag references "CrvDigi" in _eventntuples.fcl but no CrvDigi producer exists

physics.analyzers.EventNtuple.CrvDigisTag : "CrvDigi"

The standalone EventNtuples script defines CrvDigisTag : "CrvDigi" and CrvCoincidencesTag: "CrvCoincidenceClusterFinder", but its producers block only contains @table::EventNtuple.producers — it does not define CrvDigi, CrvRecoPulses, or CrvCoincidenceClusterFinder producers. This assumes the input file already contains these products under exactly those module labels. If the input was produced with different module labels (or if the products aren't in the input file), the job will fail. This is likely intentional (running on reco output), but should be documented clearly.

5. 🟢 Large code duplication between _combined.fcl and original CrvPass1_extracted.fcl

The CrvPass1_extracted_combined.fcl file duplicates nearly the entire content of CrvPass1_extracted.fcl (the full sectorConfig block, all service configurations, the producer definitions, etc.) plus the EventNtuple additions. This means any future change to CRV sector configs, DB files, or service settings must be made in multiple places. Consider using #include to pull in the base CrvPass1_extracted.fcl and then layering the EventNtuple additions on top, reducing maintenance burden.

6. 🟢 Removal of useSubsystem0 — verify upstream compatibility

The useSubsystem0 : false parameter is removed from both CrvCalibration_extracted.fcl and CrvPedestal_extracted.fcl. This is presumably because the CrvDigisFromArtdaqFragmentsFEBII module no longer supports or needs this parameter. This is fine if the upstream module has been updated accordingly, but worth confirming the Offline/CRVResponse version in use no longer expects this parameter.

---

Summary Table

#	Severity	File	Issue
1	🔴 High	_combined.fcl	Duplicate process_name : CrvPass1 will conflict with original
2	🟡 Medium	_combined.fcl	TFileService output name crvRecoExtracted.root could overwrite files
3	🟡 Medium	_eventntuples.fcl	Missing ProditionsService/DbService configuration
4	🟡 Medium	_eventntuples.fcl	CrvDigisTag references producer not defined in this script
5	🟢 Low	_combined.fcl	Heavy duplication of config from CrvPass1_extracted.fcl
6	🟢 Low	Calibration/Pedestal	Removal of useSubsystem0 — verify module compatibility

[ehrlich-uva]

1. The CrvPass1_extracted.fcl and CrvPass1_extracted_eventntuples.fcl do the same thing as CrvPass1_extracted_combined.fcl except that they are split into two fcls to separate the resulting DQM data and EventNtuples into two files. Therefore, CrvPass1_extracted.fcl will never be run on the output of CrvPass1_extracted_combined.fcl (or vice versa).
2. This is not an issue.
3. The production of EventNtuples in the separated script doesn't need these settings, since it doesn't use any calibration tables.
4. These tags refer to the data products in the art file which this fcl file processes.
5. This is because the _combined.fcl is just the combination of the two other fcls.
6. This correct. This parameter was removed in Offline.