BD and QA event flag overhaul by ckierans · Pull Request #89 · cositools/nuclearizer

ckierans · 2026-01-21T01:30:00Z

I've done an overhaul on the BD and QA (quality) event flags, and MReadoutAssembly in general. I remove balloon code relics. And updated variable names so they're a bit easier to understand.

We now have 4 BD errors:

m_EnergyCalibrationError
m_StripPairingError
m_DepthCalibrationError
m_EventReconstructionError
where each error flag can come with text that describes the error in more detail, i.e. "No calibration coefficients", or "Out of Range", etc.

We now have QA flags indicating potentially suspect events, i.e. with strip hits removed below threshold, or poor quality strip pairing. These should be used elsewhere in the code whenever an event is altered (TAC cut).

… these events in revan

…updated variable names

…ReadoutAssembly

zoglauer · 2026-01-21T07:05:15Z

+  //! Set the energy-calibration-Error flag
+  void SetEnergyCalibrationError(bool Flag = true, MString Text = "") { m_EnergyCalibrationError = Flag;  m_EnergyCalibrationErrorString = Text; }
+  //! Get the energy-calibration-Error flag
+  bool IsEnergyCalibrationError() const { return m_EnergyCalibrationError; }


HasEnergyCalibrationError() - same for all below

zoglauer · 2026-01-21T07:08:59Z

+  //Track BD Flags

+  //! Set the energy-calibration-Error flag
+  void SetEnergyCalibrationError(bool Flag = true, MString Text = "") { m_EnergyCalibrationError = Flag;  m_EnergyCalibrationErrorString = Text; }


We are overwriting the string if we have multiples. Is that OK? Otherwise the error text should be a vector and we add to it.
Do we ever want to remove the error flag? If no, we don't need the "bool Flag = true". If yes, then it might be better to have a ClearEnergyCalibrationError() function?

This comment is the same for everthing below

zoglauer · 2026-01-21T07:12:01Z

+  int GetStripHitBelowThreshold_Number() const { return m_StripHitBelowThreshold_Number;}
+
+  //! Set the Reduced Chi^2 used in MultiRoundChiSquare module
+  void SetReducedChiSquare(double ReducedChiSquare) { m_ReducedChiSquare = ReducedChiSquare; }


SetStripPairingReducedChiSquare

zoglauer · 2026-01-21T07:15:25Z


+  //! Set the Quality of this Event used in Greedy Strip pairing module
+  //! TODO Change name of this variable to be more descriptive
+  void SetEventQuality(double EventQuality){ m_EventQuality = EventQuality; }


Do we still want to keep Clio's approach around?
I don't like to have 2 strip pairing quality factors...
Think about it. I am ready to jettison it...

zoglauer · 2026-01-21T07:17:24Z


            // Add the depth to the GUI histogram.
-            if (Event->IsStripPairingIncomplete()==false) {
+            if (Event->IsStripPairingError()==false) {


White space: ..() == false

The fixes to the depth calibration module will be accounted for in the Metrology PR since I did lots of formating changes there.

zoglauer · 2026-01-21T07:19:45Z

    for (unsigned int side = 0; side <=1; ++side) { // side loop
      if (StripHits[d][side].size() > MaxStripHits) {
-        Event->SetStripPairingIncomplete(true, "More than 6 hit strIps on one side");
+        Event->SetStripPairingError(true, "More than 6 hit strIps on one side");


Typo: strIps

I'll leave this for @julianmgerber to fix in his next PR since he's touching the strip pairing errors/quality flags

zoglauer · 2026-01-21T07:20:09Z


    if (StripHits[d][0].size() == 0 || StripHits[d][1].size() == 0) {
-      Event->SetStripPairingIncomplete(true, "One detector side has not strip hits");
+      Event->SetStripPairingError(true, "One detector side has not strip hits");


has no strip hits?

@julianmgerber another one for you

zoglauer · 2026-01-21T07:22:55Z

Think also about to remove the old greedy strip paring approach and the old depth calibration modules.

ckierans · 2026-02-03T15:45:33Z

I've added an issue for removing the old greedy strip pairing method since I didn't want to do too much more with this PR.

This should now address your comments @zoglauer . The only thing I wasn't sure of was the proper way to overload the << operator to print out the contents of the vectorized error messages. I've added some code to the MReadoutAssembly.cxx to do that locally, and it works well. But let me know if that should go somewhere else.

zoglauer · 2026-02-03T17:25:25Z

+  //Track BD Flags

+  //! Set the energy-calibration-Error flag
+  void SetEnergyCalibrationError(bool Flag = true, MString Text = "") { m_EnergyCalibrationError = Flag;  m_EnergyCalibrationErrorString.push_back(Text); }


Don't push an empty string

If the error flag is set to true, and strings are set - if we set the flag to false, do we want to clear all events and clear the string vector? Is there a way we want to clear the calibration error?

This holds for all the other errors below

We don't have anything in the code that would remove any of the BD or QA flags. Not sure if we ever would want that. So I don't know that we want to create the capability to clear the error flag and string until we have a use case for it.

Could this line then just be:
void SetEnergyCalibrationError(bool Flag, MString Text) { m_EnergyCalibrationError = Flag; m_EnergyCalibrationErrorString.push_back(Text); }

If we do not want to remove this flag, then I would do:
void SetEnergyCalibrationError(MString Text = "") { m_EnergyCalibrationError = true; if (Text != "") { m_EnergyCalibrationErrorString.push_back(Text); } }

Okay, got it! Thanks for the help!

zoglauer · 2026-02-03T17:27:40Z

-
-  bool m_EnergyResolutionCalibrationIncomplete;
-  MString m_EnergyResolutionCalibrationIncompleteString;
+  // Flags indicating bad events:


Please describe each flag and parameter - since people might just read the oxygen documentation

Holds for everything below

zoglauer · 2026-02-03T17:31:01Z

  delete m_Aspect;
  m_Aspect = nullptr;
-}
+}//


That // can be removed

zoglauer · 2026-02-03T17:42:51Z

+  return os;
+}
+



No.... !!!
There is a reason there is no such converter, since S can be anything.
Just do your little for loop in the Streamer

zoglauer · 2026-02-03T17:45:51Z

-    if (m_EnergyCalibrationIncompleteString != "") S<<" ("<<m_EnergyCalibrationIncompleteString<<")";
+  if (m_EnergyCalibrationError == true) {
+    S<<"BD EnergyCalibrationError";
+    if (m_EnergyCalibrationErrorString.empty() == false) S<<" ("<<m_EnergyCalibrationErrorString<<")";


You just have a space between the strings - this makes them indistinguishable.
Do it in this format, so that we can more easily distinguish the messages:
[Error Message 1] [Error Message 2]

zoglauer · 2026-02-03T17:47:00Z

Email me when I can review it again.

ckierans · 2026-02-03T19:49:45Z

@zoglauer should be good to review again. Thanks!

ckierans and others added 6 commits December 18, 2025 10:52

Updated strip hit threshold logic, and now have QA flag to not filter…

0445f37

… these events in revan

Merge branch 'develop/em' into threshold_fix

a1add80

Included better tracking for hits removed through threshold cut, and …

a8db144

…updated variable names

Merge branch 'develop/em' into threshold_fix

3ccb3b1

Overhaul on BD flag from balloon days and updated variable names in M…

a06ce65

…ReadoutAssembly

Added minor comments to MReadoutAssembly for PR

5d83719

ckierans requested a review from zoglauer January 21, 2026 01:31

ckierans linked an issue Jan 21, 2026 that may be closed by this pull request

Check all BD flags! #50

Closed

zoglauer reviewed Jan 21, 2026

View reviewed changes

Comment thread src/MModuleDepthCalibration2024.cxx Outdated

zoglauer reviewed Jan 21, 2026

View reviewed changes

Vectorized error messages to not overwrite and updated variable names

88f00d1

zoglauer reviewed Feb 3, 2026

View reviewed changes

Fixed streaming of vectorized error message

db3de79

zoglauer merged commit af52dfc into cositools:develop/em Feb 3, 2026

ckierans deleted the bad_and_qa_flags branch March 16, 2026 19:02

Conversation

ckierans commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zoglauer Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoglauer commented Jan 21, 2026

Uh oh!

ckierans commented Feb 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoglauer commented Feb 3, 2026

Uh oh!

ckierans commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zoglauer Jan 21, 2026 •

edited

Loading