ADD: Full suite of unit tests by zoglauer · Pull Request #247 · cositools/nuclearizer

zoglauer · 2026-10-18T18:11:28Z

*** MEGAlib update required ***

This PR drops the full unit test suite for nuclearizer.
All tests are a collaboration between Claude Opus 5.5, GPT-6.1-Sol, and Andreas.

The tests uncovered 60-70 bugs where 25-35 affect data correctness. I (or probably Claude) will add issues for them next.

Lot's of tests fail. Those are marked with ExpectedToFail() (161 checks): they are reported as KNOWN FAILURE without failing the run, and as UNEXPECTED PASS once fixed failing the run. I made it that way so that github is happy while we fix the bugs.

This PR adds ~20,000 !! lines of code and ~2,400 tests -- I absolutely do not expect anybody to review it. The PR is intentionally made in a way that 0 production code is touched.

zoglauer · 2026-10-18T18:23:53Z

Just check with Arch - and getting some crashes - investigating

fhagemann · 2026-10-18T19:08:30Z

Let us know once you're done troubleshooting and want us to look at this :)

zoglauer · 2026-10-18T19:13:25Z

Different things fail on different OSes...

zoglauer · 2026-10-18T19:25:55Z

Everything is working on macOS Tahoe, Ubuntu 24.05, and the latest Arch - it is just not working here...

zoglauer · 2026-10-18T21:21:06Z

All checks passed! Ready of review!

fhagemann · 2026-10-20T03:28:05Z

Tested this locally on my laptop and all tests pass.
How should we deal with the unit tests that were added in #239 and now cause merge conflicts?

…added in a different PR

zoglauer · 2026-10-20T05:33:20Z

@fhagemann I fixed all issues and dropped my duplicate unit test

fhagemann

Disclaimer: I didn't look through the DEE submodules yet, because I want to give them a closer look when tackling the new issues that were created as a result of this.

I had a very quick look through all other unittests for files/modules that I have worked with before (not by any means a thorough review, just a quick overlook) and commented/flagged some things.

Let me know if I should look through the DEE submodule unit tests BEFORE merging this or only when tackling the issues assigned to me.

fhagemann · 2026-10-20T18:07:01Z

How does this file relate to the unit tests added in #191 ?

I will redo #191 - the issues associates to Assemble should already be fixed.

fhagemann · 2026-10-20T18:08:23Z

+    MString Input = MString("module ") + M->GetXmlTag();
+    Passed = EvaluateTrue("MAssembly()", Input, "The XML tag of the module is unique", Tags.insert(M->GetXmlTag()).second) && Passed;
+    Passed = EvaluateTrue("MAssembly()", Input, "The name of the module is unique", Names.insert(M->GetName()).second) && Passed;
+    Passed = EvaluateFalse("MAssembly()", Input, "The XML tag is not empty", M->GetXmlTag().IsEmpty()) && Passed;
+    Passed = EvaluateTrue("MAssembly()", Input, "The module provides at least one module type", M->GetNModuleTypes() > 0) && Passed;
+  }
+  Passed = EvaluateTrue("MAssembly()", "available modules", "All available modules are non-null", AllValid) && Passed;


Not sure if we should make this a requirement, but add a test to check if the XML tag starts with XmlTag (following the discussion in #193 / #197 ) ?

Let me think about that.

fhagemann · 2026-10-20T18:11:28Z

+A test must never read or write the user's files.  The `MUnitTest`
+constructor therefore creates a private temporary directory with a
+randomized name (e.g. `MEGAlib_aB3xYz9Qp1_home`) inside its temporary root,
+points `HOME` to it, and changes the working directory into it.  Neither is


About pointing HOME somewhere else for the unittests:
When running the end-to-end tests for 406-1 and 542-1, they are still generating the cross-sections directories in resource/unittestdata when running this locally(zoglauer/megalib#123). Can this somehow be avoided by having the unit tests create those directories also in a temporary directory instead?

I know. That requires MEGAlib changes.

fhagemann · 2026-10-20T19:07:11Z

+////////////////////////////////////////////////////////////////////////////////
+
+
+bool UTNModuleTrappingCorrection::TestInterpolate()


I feel like the Interpolate function we have in MModuleChargeTrapping right now might be useful elsewhere, and might therefore live in some utility module instead (how to we interpolate CTD-to-depth splines in the depth calibration module)?

Sounds good with me.

fhagemann · 2026-10-20T19:16:30Z

I don't think we need unit tests for the template ^^
Is the template compiled when running make - if so, we might want to update the Makefile to not compile/test it.

fhagemann · 2026-10-22T06:11:41Z

+    Passed = EvaluateNear("AnalyzeEvent()", "LV strip 10", "The timing is 4200 - 1200 = 3000 ns", L10.m_Timing, 3000.0, 1e-9) && Passed;
+    Passed = Evaluate("AnalyzeEvent()", "LV strip 10", "The TAC is (3000 + 500) / 0.5 = 7000", L10.m_TAC, 7000u) && Passed;


I guess it's documented here, but as said: not sure if this is the final configuration we want in the long run.

fhagemann · 2026-10-22T06:20:17Z

+{
+  bool Passed = true;
+
+  // The TAC calibration only covers LV strips 32-63 of detector 0, as in the committed unit-level calibrations


????

TAC calibration should cover all strips
This might be due to the strip map, where STRIP_ID 0-31 are LV strips 32-63, but then followed by STRIP_ID 32-63, which are LV strips 0-31. I'm not sure we EVER expect partial TAC calibrations (?)

fhagemann · 2026-10-22T06:23:20Z

+MString UTNSubModuleStripReadout::WriteThresholds(unsigned int NStrips)
+{
+  // Hardware threshold 1600 ADC (800 keV) for all strips
+  MString C = "STRIP_ID,Detector,side,strip,threshold_adc,threshold_keV\n";
+  for (unsigned int s = 0; s < NStrips; ++s) {
+    C += MString("") + s + ",0,l," + s + ",1600,800.0\n";
+    C += MString("") + (100 + s) + ",0,h," + s + ",1600,800.0\n";
+  }
+  MString FileName = GetTemporaryFileName("sr.thresholds.csv");
+  if (WriteTextFile(FileName, C) == false) return "";
+  return FileName;
+}


I expect the hardware thresholds for the DEE to have the same issue as what was fixed for the TAC cal in #239
--> we might need to apply the strip map also to the hardware threshold file or make sure that the Detector/side/strip notation is correct.

fhagemann · 2026-10-22T06:35:39Z

+  }
+
+  // FWHM at 1000 keV is 3 keV => sigma = 3 / 2.355
+  gRandom->SetSeed(4711);


Is there something special about the random seed 4711? 😅

fhagemann · 2026-10-22T06:38:36Z

This module might go anyway

fhagemann

I'm done reviewing :)

fhagemann · 2026-10-22T22:39:03Z

You commented „Done“ on some comments, but never posted anything (?) 😅

zoglauer · 2026-10-27T04:00:43Z

I addressed most of the meant to be addressed comments I think.

fhagemann

Looks great, thanks so much!

fhagemann · 2026-10-27T12:55:52Z

+unsigned int UTNSubModuleDepthReadout::GetDIB(unsigned int DetID) const
+{
+  // Deliberately different from the detector ID: the DIB column of the TAC calibration file must not be used as detector ID
+  return DetID + 4;
 }


Nice, thanks!

ADD: Full suite of unit tests

1f3731e

zoglauer requested review from ckierans and fhagemann October 18, 2026 18:11

zoglauer self-assigned this Oct 18, 2026

zoglauer added 2 commits October 18, 2026 11:37

CHG: Work around differences of OS, compiler & libraries

1440e5b

CHG: More work arounds for differences of OS, compiler & libraries

91be40b

zoglauer and others added 3 commits October 18, 2026 12:56

CHG: Always show failures

64950b1

CHG: Speed up the responegenerator test

8dd1a4c

CHG: Handle different behaviour with and without DISPLAY

60d0312

zoglauer added 3 commits October 19, 2026 22:05

CHG: Removed unittests/UTNModuleTACCalibration.cxx since it has been …

881b1de

…added in a different PR

MRG: develop/em

8f1bded

CHG: Chnages for recent commits

67ec34c

fhagemann reviewed Oct 20, 2026

View reviewed changes

fhagemann added the test Tests and code coverage label Oct 20, 2026

fhagemann reviewed Oct 22, 2026

View reviewed changes

fhagemann approved these changes Oct 22, 2026

View reviewed changes

fhagemann mentioned this pull request Oct 24, 2026

FIX: Issues in the DEE module and its submodule #290

Open

CHG: Addressed select comments of Felix

4e5313b

zoglauer merged commit 6c37249 into cositools:develop/em Oct 27, 2026
1 check passed

fhagemann reviewed Oct 27, 2026

View reviewed changes

This was referenced Oct 27, 2026

CHG: MAssembly hardening and unit tests #191

Closed

Create a multi-detector unit test #292

Open

		////////////////////////////////////////////////////////////////////////////////


		bool UTNModuleTrappingCorrection::TestInterpolate()

		Passed = EvaluateNear("AnalyzeEvent()", "LV strip 10", "The timing is 4200 - 1200 = 3000 ns", L10.m_Timing, 3000.0, 1e-9) && Passed;
		Passed = Evaluate("AnalyzeEvent()", "LV strip 10", "The TAC is (3000 + 500) / 0.5 = 7000", L10.m_TAC, 7000u) && Passed;

Conversation

zoglauer commented Oct 18, 2026

Uh oh!

zoglauer commented Oct 18, 2026

Uh oh!

fhagemann commented Oct 18, 2026

Uh oh!

zoglauer commented Oct 18, 2026

Uh oh!

zoglauer commented Oct 18, 2026

Uh oh!

zoglauer commented Oct 18, 2026

Uh oh!

fhagemann commented Oct 20, 2026

Uh oh!

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

Uh oh!

Uh oh!

fhagemann left a comment

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!

fhagemann Oct 20, 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!

zoglauer Oct 22, 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fhagemann Oct 22, 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!

fhagemann left a comment

Choose a reason for hiding this comment

Uh oh!

fhagemann commented Oct 22, 2026

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

fhagemann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

zoglauer commented Oct 20, 2026 •

edited

Loading

fhagemann Oct 20, 2026 •

edited

Loading

zoglauer Oct 22, 2026 •

edited

Loading

fhagemann Oct 22, 2026 •

edited

Loading

zoglauer commented Oct 27, 2026 •

edited

Loading