Set the EWT to be the event number in sims#1807
Set the EWT to be the event number in sims#1807michaelmackenzie wants to merge 2 commits intoMu2e:mainfrom
Conversation
|
Hi @michaelmackenzie,
which require these tests: build. @Mu2e/fnalbuild-users, @Mu2e/write have access to CI actions on main. ⌛ The following tests have been triggered for 32aa705: build (Build queue - API unavailable) |
PR #1807 Review — Set the EWT to be the event number in simsSummaryAuthor: @michaelmackenzie Intent (from PR body): Allow the EWT to vary in simulations so that lumi-stream tests see a linearly increasing EWT inside a subrun. The full (run/subrun/event) tuple can't be packed into the 64-bit EWT, so the event number alone is used as a best-effort proxy. Core ChangeFile: if(!event.getByLabel(cfoFragmentTag_, cfoFragmentHandle)) {
+ // Set the EWT to be the art event number for now, useful for simulations
+ evtHdr->ewt = static_cast<long unsigned int>(event.event());
event.put(std::move(evtHdr));
if(ewm_) event.put(std::move(ewm));
TLOG(TLVL_DEBUG + 1) << "No CFO fragments found";
return;
}Previously, when there was no CFO fragment collection in the event (the sim path), 🔎 Issue found — inconsistent handling of the "no CFO fragments" caseThere are actually two places in if(!event.getByLabel(cfoFragmentTag_, cfoFragmentHandle)) {
// Set the EWT to be the art event number for now, useful for simulations
evtHdr->ewt = static_cast<long unsigned int>(event.event()); // ← patched
event.put(std::move(evtHdr));
if(ewm_) event.put(std::move(ewm));
TLOG(TLVL_DEBUG + 1) << "No CFO fragments found";
return;
}
const auto *fragments = cfoFragmentHandle.product();
if (fragments->size()>0){
...
evtHdr->ewt = static_cast<long unsigned int>(cfo.GetEventWindowTag()...);
...
} else {
TLOG(TLVL_DEBUG + 3) << "No CFO fragments found in the event"; // ← NOT patched, ewt stays default
}If the handle is present but the fragment vector is empty ( Suggested fix — set the fallback EWT for both "no fragment" branches, e.g.: // default EWT for the sim / no-fragment case
evtHdr->ewt = static_cast<long unsigned int>(event.event());
if(!event.getByLabel(cfoFragmentTag_, cfoFragmentHandle)) {
event.put(std::move(evtHdr));
if(ewm_) event.put(std::move(ewm));
TLOG(TLVL_DEBUG + 1) << "No CFO fragments found";
return;
}
const auto *fragments = cfoFragmentHandle.product();
if (fragments->size() > 0) {
...
evtHdr->ewt = static_cast<long unsigned int>(cfo.GetEventWindowTag()...); // overwrite with real value
...
} else {
TLOG(TLVL_DEBUG + 3) << "No CFO fragments found in the event";
// evtHdr->ewt already seeded from event.event()
}This keeps the single-line-of-intent semantics and ensures both "simish" code paths produce a useful EWT. Other observations / possible improvements
Merge readiness
Want me to…
|
|
If there is a CFO fragment collection in the event, but no usable CFO fragment, I don't think this should be handled like simulation and set to a non-zero value. I think it's better to leave this alone and perhaps later we'll throw an error or filter out this event due to missing CFO or corrupted information. |
|
☀️ The build tests passed at 32aa705.
N.B. These results were obtained from a build of this Pull Request at 32aa705 after being merged into the base branch at 314d115. For more information, please check the job page here. |
|
@FNALbuild run build test |
|
⌛ The following tests have been triggered for 5a0d718: build (Build queue - API unavailable) |
|
☀️ The build tests passed at 5a0d718.
N.B. These results were obtained from a build of this Pull Request at 5a0d718 after being merged into the base branch at 314d115. For more information, please check the job page here. |
|
📝 The HEAD of |
3 participants
This will at least allow for changing EWT values in simulations. The run/subrun/event values can't be packed into the 64 bit EWT, so this cannot all be encoded. The linearly increasing EWT values within a subrun will be useful for lumi stream tests, where the subrun number can be taken from the art event but the event number is lost in the buffering of objects.