Skip to content

CHG: MAssembly fixes: #251, #252, #253, #254 - #294

Merged
zoglauer merged 3 commits into
cositools:develop/emfrom
zoglauer:feature/massembly-unittest-redo
Oct 28, 2026
Merged

zoglauer merged 3 commits into
cositools:develop/emfrom
zoglauer:feature/massembly-unittest-redo

Conversation

@zoglauer

@zoglauer zoglauer commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

This commit is just command line parsing and error handling - nothing critical in the analysis path

Comment thread include/MAssembly.h
Comment thread src/MAssembly.cxx
Comment on lines 191 to +192
Usage<<" -m --multithreading:"<<endl;
Usage<<" 0: false (default), else: true"<<endl;
Usage<<" 0: false, else: true (default)"<<endl;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What's the default number of threads used? Worth mentioning here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

One thread per module, more for paralleizable slow ones.
I will update the comment

Comment thread src/MAssembly.cxx
Comment on lines +261 to +265
if (Verbosity < c_Quiet || Verbosity > c_Info) {
cout<<"ERROR: Command-line parser: Option "<<Option<<" needs an argument between "<<c_Quiet<<" and "<<c_Info<<", not \""<<Value<<"\"!"<<endl;
cout<<Usage.str()<<endl;
return 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Repeating comment from #191 (comment):
Currently, we flush the terminal output with c_Info statements that are being printed for every Event, e.g. in AnalyzeEvent of the energy calibration. We might want to think about adding also MEGAlib c_Chatty option here, and always avoid having anything that's not c_Chatty in AnalyzeEvents or loop iterating through all Events or (strip) hits.
But this doesn't have to be in this PR.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

--> #295

Comment thread src/MAssembly.cxx

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ I checked that this addresses the issues:

Comment thread unittests/UTNAssembly.cxx Outdated
Comment on lines +280 to +285
vector<MString> XmlTags = { "XmlTagLoaderSimulations", "XmlTagMeasurementLoaderROA", "XmlTagMeasurementLoaderTRA", "XmlTagMeasurementLoaderHDF",
"XmlTagMeasurementLoaderFITS", "XmlTagMeasurementLoaderL0", "XmlTagDEESMEX", "XmlTagEventFilter",
"XmlTagEnergyCalibration", "XmlTagStripPairingMultiRoundChiSquare", "XmlTagStripPairingChiSquare",
"XmlTagDepthCalibration", "XmlTagTrappingCorrection", "XmlTagEventSaver", "XmlTagSaverMeasurementsL0",
"XmlTagTransmitterRealta", "XmlTagResponseGenerator", "XmlTagRevan", "XmlTagTACCalibration",
"XmlTagDiagnostics", "XmlTagDiagnosticsEnergyPerStrip" };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a way to generate this from code rather that having a hard-coded list here, for when we add more modules to nuclearizer (e.g. the lollipop backscatter module in #246)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Then it would not be an independent test anymore...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see: as long as the unit tests send some sort of reminder (which seems to be the case by comparing the number of listed modules), this should be fine :)

Comment thread unittests/UTNAssembly.cxx Outdated
Comment thread unittests/UTNAssembly.cxx
Comment on lines +474 to +479
CallParseCommandLine(A, { "-c", Empty, "-m", "1", "-t" }, Output);
Passed = EvaluateTrue("ParseCommandLine()", "-m 1", "-m 1 enables multithreading", Output.Contains("Command-line parser: Using multithreading: yes")) && Passed;
CallParseCommandLine(A, { "-c", Empty, "-m", "2", "-t" }, Output);
Passed = EvaluateTrue("ParseCommandLine()", "-m 2", "Any non-zero value enables multithreading", Output.Contains("Command-line parser: Using multithreading: yes")) && Passed;
CallParseCommandLine(A, { "-c", Empty, "--multithreading", "0", "-t" }, Output);
Passed = EvaluateTrue("ParseCommandLine()", "--multithreading 0", "--multithreading 0 disables multithreading", Output.Contains("Command-line parser: Using multithreading: no")) && Passed;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also check that the default multithreading is true?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I am missing a Get'er in MSupervisor...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it worth adding this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Did it - new you have to update MEGAlib

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants