Reduce dependence on HistogramTools

[sdobbs]

We've seen that when using newer versions of gcc and/or ROOT, that sometimes we run into segfaults that seem to be linked to the HistogramTools package. Also, even though it's convenient, its locking behavior is not the most efficient.

So this is a first attempt to rewrite plugins to remove the dependence on this header.

I suggest that this PR not be merged until after the next tagged release
I've checked to make sure that the new plugins run correctly, but haven't done more detailed checks - this PR can provide a starting point for those.

[gluex]

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/summary.txt
Logs: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/log

Build log: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/make_sdobbs_remove_histogramtools.log
Build report: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/report_sdobbs_remove_histogramtools.txt
Location of build: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools

[aaust]

The plugin BCAL_ADC_4ns is not included in the SConscript file, and it does not compile either.
@markdalton Is the plugin still in use, and is it worth fixing?

[sdobbs]

For what it's worth, that plugin uses the old-style DReferenceTrajectory track extrapolations, which causes the compilation problems. Also, we haven't had to worry about the 4 ns shifts this plugin corrects for in the past 4 years or so, so it's probably okay to not fix it right now.

[aaust]

Ok thanks, I will test all other plugins.

[aaust]

• TrackingPulls crashes in the first 30 events
• CDC_Efficiency seems to be stuck in an infinite loop

The remaining list of plugins is running with a similar speed as the master:

• CDC_PerStrawReco
• FDC_Efficiency
• CDC_TimeToDistance
• BCAL_SiPM_saturation
• BCAL_attenlength_gainratio
• FCALLEDTree (needs -PFCALLED:Tree=1 to run, should be fixed)

The individual histograms still have to be validated.

[sdobbs]

Thanks for checking - how did you run these? Just one plugin at a time?
I tried mostly running all of them together, but these concurrency problems can show up in odd ways.

[aaust]

All plugins were run together, but there should not be any interference afaict.
I corrected and validated FDC_Efficiency and TrackingPulls.

[sdobbs]

OK, thanks for fixing those.

[gluex]

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/summary.txt
Logs: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/log

Build log: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/make_sdobbs_remove_histogramtools.log
Build report: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/report_sdobbs_remove_histogramtools.txt
Location of build: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools

[aaust]

This lock is responsible for the "infinite loop". It is opened inside an if statement:

halld_recon/src/plugins/monitoring/CDC_Efficiency/JEventProcessor_CDC_Efficiency.cc

Lines 561 to 576 in 9472603

	if(foundHit)
	{
	const DCDCDigiHit *thisDigiHit = NULL;
	const Df125CDCPulse *thisPulse = NULL;
	locHit->GetSingle(thisDigiHit);
	if (thisDigiHit != NULL)
	thisDigiHit->GetSingle(thisPulse);
	if (thisPulse != NULL)
	{
	ROCIDFromRingStraw[ringNum - 1][wireNum - 1] = thisPulse->rocid;
	SlotFromRingStraw[ringNum - 1][wireNum - 1] = thisPulse->slot;
	ChannelFromRingStraw[ringNum - 1][wireNum - 1] = thisPulse->channel;
	}
	japp->RootFillLock(this); //ACQUIRE ROOT FILL LOCK

but released outside of it:

halld_recon/src/plugins/monitoring/CDC_Efficiency/JEventProcessor_CDC_Efficiency.cc

Line 648 in 9472603

japp->RootFillUnLock(this); //RELEASE ROOT FILL LOCK

[sdobbs]

OK, I pushed a fix for CDC_Efficiency. I don't think FCALLEDTree needs to be changed - by default it just fills some histograms, so I think it's up to the original author(s) to say if they want the default behavior changed.

[gluex]

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/summary.txt
Logs: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/log

Build log: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/make_sdobbs_remove_histogramtools.log
Build report: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/report_sdobbs_remove_histogramtools.txt
Location of build: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools

[aaust]

@sdobbs Thanks. I missed that you actually fixed FCALLEDTree in one of your commits.

[aaust]

Why is this commented out?

halld_recon/src/plugins/monitoring/CDC_PerStrawReco/JEventProcessor_CDC_PerStrawReco.cc

Lines 503 to 532 in fc1a18f

	/*
	// fill per-straw histograms
	hStrawDriftTimeVsPhiDOCA[region][ring-1][straw-1]->Fill(docaphi, time);
	hStrawPredictedDistanceVsPhiDOCA[region][ring-1][straw-1]->Fill(docaphi, predictedDistance);
	hStrawResidual[region][ring-1][straw-1]->Fill(residual);
	hStrawResidualVsZ[region][ring-1][straw-1]->Fill(dz,residual);
	//Time to distance relation in bins
	// Calcuate delta
	double delta = max_sag[ring - 1][straw - 1]*(1.-dz*dz/5625.)
	*cos(docaphi + sag_phi_offset[ring - 1][straw - 1]);
	if ( 2 * max_sag[ring - 1][straw - 1] > binwidth){
	hStrawResidualVsDelta[region][ring-1][straw-1]->Fill(delta, residual);
	}
	hStrawPredictedDriftDistanceVsDriftTime[region][ring-1][straw-1]->Fill(time, predictedDistance);
	hStrawPredictedDriftDistanceVsDelta[region][ring-1][straw-1]->Fill(delta, predictedDistance - 0.78);
	if (delta > 0){ // Long side of straw
	hPredictedDriftDistanceVsDriftTime_PosDelta[region][ring-1][straw-1]->Fill(time, predictedDistance);
	hResidualVsDriftTime_PosDelta[region][ring-1][straw-1]->Fill(time, residual);
	hResidual_PosDelta[region][ring-1][straw-1]->Fill(residual);
	}
	else { // Short side of straw
	hPredictedDriftDistanceVsDriftTime_NegDelta[region][ring-1][straw-1]->Fill(time, predictedDistance);
	hPredictedDriftDistanceVsDriftTime_NegDelta[region][ring-1][straw-1]->Fill(time, residual);
	hResidual_NegDelta[region][ring-1][straw-1]->Fill(residual);
	}
	*/

[sdobbs]

That's a mistake which slipped in during debugging some of these other problems - those lines should be uncommented.

[gluex]

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/summary.txt
Logs: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/log

Build log: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/make_sdobbs_remove_histogramtools.log
Build report: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/report_sdobbs_remove_histogramtools.txt
Location of build: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools

[gluex]

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/summary.txt
Logs: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/log

Build log: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/make_sdobbs_remove_histogramtools.log
Build report: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/report_sdobbs_remove_histogramtools.txt
Location of build: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools

[gluex]

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/summary.txt
Logs: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/log

Build log: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/make_sdobbs_remove_histogramtools.log
Build report: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/report_sdobbs_remove_histogramtools.txt
Location of build: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools

[gluex]

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/summary.txt
Logs: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/tests/log

Build log: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/make_sdobbs_remove_histogramtools.log
Build report: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools/report_sdobbs_remove_histogramtools.txt
Location of build: /work/halld/pull_request_test/halld_recon^sdobbs_remove_histogramtools

[aaust]

Thoroughly tested. The only obvious differences that remain concern histogram types, e.g. TH1I instead of TH1F or similar.