Skip to content

Trigger simulation modules now support input tags with process name [private] #870

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.

Sign up for GitHub

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

Merged
mvicenzi merged 3 commits into from
Nov 19, 2025
Merged

Trigger simulation modules now support input tags with process name [private] #870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9667657
Tell CMake of sbnalg dependency
PetrilloAtWork Nov 18, 2025
8c1e40b
Trigger simulation modules: support input tag with process name
PetrilloAtWork Nov 18, 2025
937b4b6
Apply suggestions from GIT Copilot code review
PetrilloAtWork Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ find_package(ifdhc REQUIRED )
find_package(ifdh_art REQUIRED )
find_package(Range-v3 REQUIRED )
find_package(sbnobj REQUIRED )
find_package(sbnalg REQUIRED )
find_package(icarus_signal_processing REQUIRED )
find_package(icarusalg REQUIRED )
find_package(icarusutil REQUIRED )
Expand Down
40 changes: 22 additions & 18 deletions icaruscode/PMT/Trigger/LVDSgates_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ class icarus::trigger::LVDSgates: public art::EDProducer {
{ ComboMode::OR, "OR" }
};

fhicl::OptionalAtom<std::string> TriggerGatesTag {
fhicl::OptionalAtom<art::InputTag> TriggerGatesTag {
Name("TriggerGatesTag"),
Comment("label of trigger gate extraction module (no instance name)")
Comment("tag of trigger gate extraction module (no instance name)")
};

fhicl::Sequence<std::string> Thresholds {
Expand Down Expand Up @@ -386,11 +386,11 @@ class icarus::trigger::LVDSgates: public art::EDProducer {

/// Converts a threshold string into an input tag.
static art::InputTag makeTag
(std::string const& thresholdStr, std::string const& defModule);
(std::string const& thresholdStr, art::InputTag const& defModule);

/// Converts an input tag into an instance name for the corresponding output.
static std::string makeOutputInstanceName
(art::InputTag const& inputTag, std::string const& defModule);
(art::InputTag const& inputTag, art::InputTag const& defModule);

}; // class icarus::trigger::LVDSgates

Expand All @@ -412,11 +412,11 @@ icarus::trigger::LVDSgates::LVDSgates
//
// more complex parameter parsing
//
std::string const discrModuleLabel = config().TriggerGatesTag().value_or("");
art::InputTag const discrModuleTag = config().TriggerGatesTag().value_or("");
for (std::string const& thresholdStr: config().Thresholds()) {
art::InputTag const inputTag = makeTag(thresholdStr, discrModuleLabel);
art::InputTag const inputTag = makeTag(thresholdStr, discrModuleTag);
fADCthresholds[thresholdStr]
= { inputTag, makeOutputInstanceName(inputTag, discrModuleLabel) };
= { inputTag, makeOutputInstanceName(inputTag, discrModuleTag) };
} // for all thresholds

//
Expand Down Expand Up @@ -863,34 +863,38 @@ auto icarus::trigger::LVDSgates::ReadTriggerGates(

//------------------------------------------------------------------------------
art::InputTag icarus::trigger::LVDSgates::makeTag
(std::string const& thresholdStr, std::string const& defModule)
(std::string const& thresholdStr, art::InputTag const& defModule)
{
auto const isNumber = [pattern=std::regex{ "[+-]?[0-9]+" }]
(std::string const& s) -> bool
{ return std::regex_match(s, pattern); };
return
if (!thresholdStr.empty() && !defModule.instance().empty()) {
throw art::Exception(art::errors::Configuration)
<< "Both the input gate module tag instance name (`TriggerGatesTag`: '"
<< defModule.encode() << "') and the threshold '" << thresholdStr
<< "' are set. One of them must be empty.\n";
}
return
((thresholdStr.find(':') != std::string::npos) || !isNumber(thresholdStr))
? art::InputTag{ thresholdStr }
: defModule.empty()
: defModule.label().empty()
? throw (art::Exception(art::errors::Configuration)
<< "No default module label (`TriggerGatesTag`) specified"
", and it's needed for threshold '"
<< thresholdStr << "'.\n")
: art::InputTag{ defModule, thresholdStr }
", and it's needed for threshold '" << thresholdStr << "'.\n")
: art::InputTag{ defModule.label(), thresholdStr, defModule.process() }
;
} // icarus::trigger::LVDSgates::makeTag()


//------------------------------------------------------------------------------
std::string icarus::trigger::LVDSgates::makeOutputInstanceName
(art::InputTag const& inputTag, std::string const& defModule)
(art::InputTag const& inputTag, art::InputTag const& defModule)
{
return (inputTag.label() == defModule)
return (inputTag.label() == defModule.label())
? inputTag.instance()
: inputTag.instance().empty()
? inputTag.label(): inputTag.label() + inputTag.instance()
: inputTag.label() + inputTag.instance()
;
Comment on lines +895 to 896
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The logic has changed from the original. Previously, when inputTag.label() didn't match defModule, it would return either inputTag.label() if the instance was empty, or inputTag.label() + inputTag.instance() if the instance was not empty. Now it always returns inputTag.label() + inputTag.instance(), which means when instance is empty, it will just return the label. While this may be the intended behavior, this change in logic should be carefully verified to ensure it doesn't break existing functionality, especially since the concatenation logic has been simplified.

Suggested change
: inputTag.label() + inputTag.instance()
;
: (inputTag.instance().empty()
? inputTag.label()
: inputTag.label() + inputTag.instance());

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@PetrilloAtWork PetrilloAtWork Nov 19, 2025

Choose a reason for hiding this comment

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

The code appears to me fully equivalent, still returning inputTag.label() + inputTag.instance() if the instance is not empty, and inputTag.label() if the instance is empty (in which case it is the same as inputTag.label() + inputTag.instance()).

} // icarus::trigger::LVDSgates::makeTag()
} // icarus::trigger::LVDSgates::makeOutputInstanceName()


//------------------------------------------------------------------------------
Expand Down
27 changes: 23 additions & 4 deletions icaruscode/PMT/Trigger/MajorityTriggerSimulation_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,9 @@ class icarus::trigger::MajorityTriggerSimulation
using Name = fhicl::Name;
using Comment = fhicl::Comment;

fhicl::Atom<std::string> TriggerGatesTag {
fhicl::Atom<art::InputTag> TriggerGatesTag {
Name("TriggerGatesTag"),
Comment("label of the input trigger gate data product (no instance name)")
Comment("tag of the input trigger gate data product")
};

fhicl::Sequence<raw::ADC_Count_t> Thresholds {
Expand Down Expand Up @@ -597,6 +597,11 @@ class icarus::trigger::MajorityTriggerSimulation
//@}


/// Returns `defModule` with instance name replaced by `thresholdStr`.
static art::InputTag makeTag
(art::InputTag const& defModule, std::string const& thresholdStr);


}; // icarus::trigger::MajorityTriggerSimulation


Expand Down Expand Up @@ -629,10 +634,10 @@ icarus::trigger::MajorityTriggerSimulation::MajorityTriggerSimulation
//
// more complex parameter parsing
//
std::string const discrModuleLabel = config().TriggerGatesTag();
art::InputTag const& discrModuleTag = config().TriggerGatesTag();
for (raw::ADC_Count_t threshold: config().Thresholds()) {
fADCthresholds[icarus::trigger::ADCCounts_t{threshold}]
= art::InputTag{ discrModuleLabel, util::to_string(threshold) };
= makeTag(discrModuleTag, util::to_string(threshold));
}

// initialization of a vector of atomic is not as trivial as it sounds...
Expand Down Expand Up @@ -943,6 +948,20 @@ icarus::trigger::MajorityTriggerSimulation::triggerInfoToTriggerData
} // icarus::trigger::MajorityTriggerSimulation::triggerInfoToTriggerData()


//------------------------------------------------------------------------------
art::InputTag icarus::trigger::MajorityTriggerSimulation::makeTag
(art::InputTag const& defModule, std::string const& thresholdStr)
{
if (!thresholdStr.empty() && !defModule.instance().empty()) {
throw art::Exception(art::errors::Configuration)
<< "Module tag instance name (`TriggerGatesTag`: '"
<< defModule.encode() << "') and the threshold '" << thresholdStr
<< "' are both set. One of them must be empty.\n";
}
return { defModule.label(), thresholdStr, defModule.process() };
} // icarus::trigger::MajorityTriggerSimulation::makeTag()


//------------------------------------------------------------------------------
DEFINE_ART_MODULE(icarus::trigger::MajorityTriggerSimulation)

Expand Down
29 changes: 24 additions & 5 deletions icaruscode/PMT/Trigger/SlidingWindowTriggerSimulation_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,9 @@ class icarus::trigger::SlidingWindowTriggerSimulation
using Name = fhicl::Name;
using Comment = fhicl::Comment;

fhicl::Atom<std::string> TriggerGatesTag {
fhicl::Atom<art::InputTag> TriggerGatesTag {
Name("TriggerGatesTag"),
Comment("label of the input trigger gate data product (no instance name)")
Comment("tag of the input trigger gate data product (no instance name)")
};

fhicl::Sequence<std::string> Thresholds {
Expand Down Expand Up @@ -573,7 +573,12 @@ class icarus::trigger::SlidingWindowTriggerSimulation
static double eventTimestampInSeconds(art::Timestamp const& time);
static double eventTimestampInSeconds(art::Event const& event);
//@}



/// Returns `defModule` with instance name replaced by `thresholdStr`.
static art::InputTag makeTag
(art::InputTag const& defModule, std::string const& thresholdStr);

}; // icarus::trigger::SlidingWindowTriggerSimulation


Expand Down Expand Up @@ -622,9 +627,9 @@ icarus::trigger::SlidingWindowTriggerSimulation::SlidingWindowTriggerSimulation
//
// more complex parameter parsing
//
std::string const& discrModuleLabel = config().TriggerGatesTag();
art::InputTag const& discrModuleTag = config().TriggerGatesTag();
for (std::string const& threshold: config().Thresholds())
fADCthresholds[threshold] = art::InputTag{ discrModuleLabel, threshold };
fADCthresholds[threshold] = makeTag(discrModuleTag, threshold);

// initialization of a vector of atomic is not as trivial as it sounds...
fTriggerCount = std::vector<std::atomic<unsigned int>>(fADCthresholds.size());
Expand Down Expand Up @@ -1204,6 +1209,20 @@ double icarus::trigger::SlidingWindowTriggerSimulation::eventTimestampInSeconds
{ return eventTimestampInSeconds(event.time()); }


//------------------------------------------------------------------------------
art::InputTag icarus::trigger::SlidingWindowTriggerSimulation::makeTag
(art::InputTag const& defModule, std::string const& thresholdStr)
{
if (!thresholdStr.empty() && !defModule.instance().empty()) {
throw art::Exception(art::errors::Configuration)
<< "Module tag instance name (`TriggerGatesTag`: '"
<< defModule.encode() << "') and the threshold '" << thresholdStr
<< "' are both set. One of them must be empty.\n";
}
return { defModule.label(), thresholdStr, defModule.process() };
} // icarus::trigger::SlidingWindowTriggerSimulation::makeTag()


//------------------------------------------------------------------------------
DEFINE_ART_MODULE(icarus::trigger::SlidingWindowTriggerSimulation)

Expand Down
26 changes: 22 additions & 4 deletions icaruscode/PMT/Trigger/SlidingWindowTrigger_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,9 @@ class icarus::trigger::SlidingWindowTrigger: public art::EDProducer {
using Comment = fhicl::Comment;


fhicl::Atom<std::string> TriggerGatesTag {
fhicl::Atom<art::InputTag> TriggerGatesTag {
Name("TriggerGatesTag"),
Comment("label of the input trigger gate data product (no instance name)")
Comment("tag of the input trigger gate data product (no instance name)")
};

fhicl::Sequence<std::string> Thresholds {
Expand Down Expand Up @@ -385,6 +385,9 @@ class icarus::trigger::SlidingWindowTrigger: public art::EDProducer {
art::Assns<TriggerGateData_t, sbn::OpDetWaveformMeta> const& assns
);

static art::InputTag makeTag
(art::InputTag const& defModule, std::string const& thresholdStr);

}; // class icarus::trigger::SlidingWindowTrigger


Expand Down Expand Up @@ -435,9 +438,9 @@ icarus::trigger::SlidingWindowTrigger::SlidingWindowTrigger
//
// more complex parameter parsing
//
std::string const discrModuleLabel = config().TriggerGatesTag();
art::InputTag const& discrModuleTag = config().TriggerGatesTag();
for (std::string const& threshold: config().Thresholds())
fADCthresholds[threshold] = art::InputTag{ discrModuleLabel, threshold };
fADCthresholds[threshold] = makeTag(discrModuleTag, threshold);

//
// configuration report (short)
Expand Down Expand Up @@ -668,6 +671,21 @@ auto icarus::trigger::SlidingWindowTrigger::ReadTriggerGates(

} // icarus::trigger::SlidingWindowTrigger::ReadTriggerGates()


//------------------------------------------------------------------------------
art::InputTag icarus::trigger::SlidingWindowTrigger::makeTag
(art::InputTag const& defModule, std::string const& thresholdStr)
{
if (!thresholdStr.empty() && !defModule.instance().empty()) {
throw art::Exception(art::errors::Configuration)
<< "Module tag instance name (`TriggerGatesTag`: '"
<< defModule.encode() << "') and the threshold '" << thresholdStr
<< "' are both set. One of them must be empty.\n";
}
return { defModule.label(), thresholdStr, defModule.process() };
} // icarus::trigger::SlidingWindowTrigger::makeTag()


//------------------------------------------------------------------------------
DEFINE_ART_MODULE(icarus::trigger::SlidingWindowTrigger)

Expand Down
18 changes: 16 additions & 2 deletions icaruscode/PMT/Trigger/TriggerEfficiencyPlotsBase.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,9 +531,9 @@ icarus::trigger::TriggerEfficiencyPlotsBase::TriggerEfficiencyPlotsBase
if (fLogEventDetails.empty()) fLogEventDetails = fLogCategory;
} // if EventDetailsLogCategory is specified

std::string const discrModuleLabel = config.TriggerGatesTag();
art::InputTag const& discrModuleTag = config.TriggerGatesTag();
for (std::string const& threshold: config.Thresholds())
fADCthresholds[threshold] = art::InputTag{ discrModuleLabel, threshold };
fADCthresholds[threshold] = makeInputTag(discrModuleTag, threshold);


if (config.EventTreeName.hasValue()) {
Expand Down Expand Up @@ -1497,4 +1497,18 @@ icarus::trigger::TriggerEfficiencyPlotsBase::makeEdepTag(
} // icarus::trigger::MakeTriggerSimulationTree::makeEdepTag()


//------------------------------------------------------------------------------
art::InputTag icarus::trigger::TriggerEfficiencyPlotsBase::makeInputTag
(art::InputTag const& defModule, std::string const& thresholdStr)
{
if (!thresholdStr.empty() && !defModule.instance().empty()) {
throw art::Exception(art::errors::Configuration)
<< "Module tag instance name (`TriggerGatesTag`: '"
<< defModule.encode() << "') and the threshold '" << thresholdStr
<< "' are both set. One of them must be empty.\n";
}
return { defModule.label(), thresholdStr, defModule.process() };
} // icarus::trigger::TriggerEfficiencyPlotsBase::makeInputTag()


//------------------------------------------------------------------------------
10 changes: 7 additions & 3 deletions icaruscode/PMT/Trigger/TriggerEfficiencyPlotsBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class icarus::trigger::details::TriggerPassCounters {

/// Registers a new pattern in the index and returns its index (unchecked).
std::size_t registerPattern(std::string const& name);

}; // icarus::trigger::details::TriggerPassCounters


Expand Down Expand Up @@ -1010,9 +1010,9 @@ class icarus::trigger::TriggerEfficiencyPlotsBase {
Comment("label of energy deposition summary data product")
};

fhicl::Atom<std::string> TriggerGatesTag {
fhicl::Atom<art::InputTag> TriggerGatesTag {
Name("TriggerGatesTag"),
Comment("label of the input trigger gate data product (no instance name)")
Comment("tag of the input trigger gate data product (no instance name)")
};

fhicl::Sequence<std::string> Thresholds {
Expand Down Expand Up @@ -1565,6 +1565,10 @@ class icarus::trigger::TriggerEfficiencyPlotsBase {
fhicl::OptionalAtom<art::InputTag> const& EnergyDepositSummaryTag
);

/// Returns `defModule` with instance name replaced by `thresholdStr`.
static art::InputTag makeInputTag
(art::InputTag const& defModule, std::string const& thresholdStr);

}; // icarus::trigger::TriggerEfficiencyPlotsBase


Expand Down
27 changes: 23 additions & 4 deletions icaruscode/PMT/Trigger/TriggerEfficiencyPlots_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1246,9 +1246,9 @@ class icarus::trigger::TriggerEfficiencyPlots: public art::EDAnalyzer {
std::vector<art::InputTag>{ "largeant:TPCActive" }
};

fhicl::Atom<std::string> TriggerGatesTag {
fhicl::Atom<art::InputTag> TriggerGatesTag {
Name("TriggerGatesTag"),
Comment("label of the input trigger gate data product (no instance name)")
Comment("tag of the input trigger gate data product (no instance name)")
};

fhicl::Sequence<raw::ADC_Count_t> Thresholds {
Expand Down Expand Up @@ -1496,6 +1496,11 @@ class icarus::trigger::TriggerEfficiencyPlots: public art::EDAnalyzer {
/// Returns a gate that is `Max()` of all the specified `gates`.
template <typename TrigGateColl>
static auto computeMaxGate(TrigGateColl const& gates);


/// Returns `defModule` with instance name replaced by `thresholdStr`.
static art::InputTag makeTag
(art::InputTag const& defModule, std::string const& thresholdStr);


}; // icarus::trigger::TriggerEfficiencyPlots
Expand Down Expand Up @@ -1533,10 +1538,10 @@ icarus::trigger::TriggerEfficiencyPlots::TriggerEfficiencyPlots
if (fLogEventDetails.empty()) fLogEventDetails = fLogCategory;
} // if EventDetailsLogCategory is specified

std::string const discrModuleLabel = config().TriggerGatesTag();
art::InputTag const& discrModuleTag = config().TriggerGatesTag();
for (raw::ADC_Count_t threshold: config().Thresholds()) {
fADCthresholds[icarus::trigger::ADCCounts_t{threshold}]
= art::InputTag{ discrModuleLabel, util::to_string(threshold) };
= makeTag(discrModuleTag, util::to_string(threshold));
}

if (config().EventTreeName.hasValue()) {
Expand Down Expand Up @@ -2467,6 +2472,20 @@ auto icarus::trigger::TriggerEfficiencyPlots::ReadTriggerGates
} // icarus::trigger::TriggerEfficiencyPlots::ReadTriggerGates()


//------------------------------------------------------------------------------
art::InputTag icarus::trigger::TriggerEfficiencyPlots::makeTag
(art::InputTag const& defModule, std::string const& thresholdStr)
{
if (!thresholdStr.empty() && !defModule.instance().empty()) {
throw art::Exception(art::errors::Configuration)
<< "Module tag instance name (`TriggerGatesTag`: '"
<< defModule.encode() << "') and the threshold '" << thresholdStr
<< "' are both set. One of them must be empty.\n";
}
return { defModule.label(), thresholdStr, defModule.process() };
} // icarus::trigger::TriggerEfficiencyPlots::makeTag()


//------------------------------------------------------------------------------
//--- EventIDTree
//------------------------------------------------------------------------------
Expand Down
Loading