Project

General

Profile

Feature #5174

Evaluate use of assert vs throwing set::exception

Added by Brian Rebel almost 7 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Immediate
Category:
-
Target version:
Start date:
01/09/2014
Due date:
% Done:

100%

Estimated time:
Experiment:
-
Co-Assignees:
Duration:

Description

There are many instances of assert being used to control code flow in LArSoft where throwing exceptions would be more appropriate.

The full list of places where assert is found in the code is below. I suggest ignoring those instances in GenFit as that is really 3rd party code. In some cases, cassert is included but never used and those includes should be removed.

CalData/CalGausHFLBNE10kt_module.cc
69: assert(h != 0);

CalData/CalGausHFLBNE35t_module.cc
71: assert(h != 0);

CalData/CalWire_module.cc
292: assert(k < kernel.size());

CalData/DeconvGausHFLBNE10kt_module.cc
69: assert(h != 0);

CalData/DeconvGausHFLBNE35t_module.cc
71: assert(h != 0);

CalData/test/FFTTest_module.cc
32: assert(h != 0);
43: assert(h != 0);
59: assert(d >= 0 && d < n);
173: assert(nx respIm->GetNbinsX());
174: assert(ny respIm->GetNbinsY());
175: assert(nx == 2); // 1=induction, 2=collection.

ClusterFinder/RStarTree/RStarTree.h
36:#include <cassert>
110: assert(1 <= min_child_items && min_child_items <= max_child_items/2);
449: assert(n_items == max_child_items + 1);
450: assert(distribution_count > 0);
451: assert(min_child_items + distribution_count-1 <= n_items);
572: assert(n_items max_child_items + 1);

DetSim/SimWire_module.cc
521: // assert(0);

EventDisplay/Ortho3DPad.cxx
7:#include &lt;cassert&gt;
90: assert(proj evd::kYZ);
115: assert(proj == evd::kYZ);
265: assert(fMSizeEntry != 0);

EventDisplay/Ortho3DView.cxx
66: assert(proj != evd::kNoProj);
67: assert(projname.size() > 0);

EventDisplay/RecoBaseDrawer.cxx
1845: assert(proj evd::kYZ);
1908: assert(proj evd::kYZ);

EventDisplay/TQPad.cxx
8:#include <cassert>
305: assert(fRecoHisto);
306: assert(fRawHisto);

Genfit/GeaneMCApplication.cxx
5:#include"assert.h"
26: //assert(field!=NULL);

Genfit/GeaneTrackRep2.cxx
26:#include <cassert>

Genfit/GFAbsEnergyLoss.h
28:#include <assert.h>

Genfit/GFAbsGeoMatManager.h
29:#include <assert.h>

Genfit/GFAbsTrackRep.cxx
21:#include <assert.h>

Genfit/GFBookkeeping.h
28:#include<cassert>

Genfit/GFDaf.cxx
31:#include "assert.h"
335: assert(expArg.GetNrows()==1);
341: assert(cutVal>1.E-6);
456: assert(b1>0);fBeta.push_back(b1);
457: assert(b2>0 && b2<=b1);fBeta.push_back(b2);
459: assert(b3<=b2);fBeta.push_back(b3);
461: assert(b4<=b3);fBeta.push_back(b4);
463: assert(b5<=b4);fBeta.push_back(b5);
465: assert(b6<=b5);fBeta.push_back(b6);
467: assert(b7<=b6);fBeta.push_back(b7);
469: assert(b8<=b7);fBeta.push_back(b8);
471: assert(b9<=b8);fBeta.push_back(b9);
473: assert(b10<=b9);fBeta.push_back(b10);

Genfit/GFDetPlane.cxx
21:#include "assert.h"
68: assert(this!=&rhs);
220: assert(fU!=fV);
234: assert(null.Mag()<1E-6);

Genfit/GFEnergyLossBetheBloch.cxx
21:#include "assert.h"
63: assert(dedx>0);
74: assert (noise!=NULL); // assert that optional argument noise exists

Genfit/GFEnergyLossBetheBloch.h
28:#include <assert.h>

Genfit/GFEnergyLossBrems.cxx
21:#include "assert.h"
172: assert(dedxBrems>=0.);
208: assert (noise!=NULL); // assert that optional argument noise exists

Genfit/GFEnergyLossBrems.h
28:#include <assert.h>

Genfit/GFEnergyLossCoulomb.cxx
21:#include "assert.h"
48: assert (noise!=NULL); // assert that optional argument noise exists
49: assert (jacobian!=NULL); // assert that optional argument jacobian exist
50: assert (directionBefore!=NULL); // assert that optional argument directionBefore exists
51: assert (directionAfter!=NULL); // assert that optional argument directionAfter exists

Genfit/GFEnergyLossCoulomb.h
28:#include <assert.h>

Genfit/GFGeoMatManager.cxx
20:#include"assert.h"
36: assert(gGeoManager->GetCurrentVolume()->GetMedium()!=NULL);
75: assert(Z>0&&Z<=MeanExcEnergy_NELEMENTS);
87: assert(fabs(index-((mix->GetZmixt())[i]))<1.e-3);
97: assert(fabs(index-mat->GetZ())<1.e-3);

Genfit/GFGeoMatManager.h
31:#include <assert.h>

Genfit/GFKalman.cxx
21:#include "assert.h"
53: assert(direction==1 || direction==-1);
259: assert(chisq.GetNoElements()==1);
298: assert(r.GetNrows()>0);

Genfit/GFMaterialEffects.cxx
97: //assert(points.size()==pointPaths.size());
240: assert(fraction>0.&&fraction<1.);
254: assert(gGeoManager->GetCurrentVolume()->GetMedium()!=NULL);
668: assert(Z>=0&&Z<MeanExcEnergy_NELEMENTS);

Genfit/GFRecoHitProducer.h
34:#include<assert.h>
120: assert(hitArrayTClones!=NULL);
121: //assert(hitArrayTClones!=NULL || hitArrayVector!=NULL);//at least one exists
122: //assert(!(hitArrayTClones!=NULL && hitArrayVector!=NULL));//but not both
132: //else{//after assertions this is save: the hitArrayVector is good

Genfit/GFRectFinitePlane.cxx
21:#include<cassert>
27:{assert(umin<umax);assert(vmin<vmax);}

Genfit/GFSpacepointHitPolicy.cxx
21:#include "assert.h"

Genfit/GFTrack.cxx
19:#include <assert.h>
67: assert(_tr.fBookkeeping.at(i)!= NULL) ;
102: assert(_tr.fBookkeeping.at(i)!= NULL) ;
177: assert(rep->getState()==getTrackRep(repid)->getState());
227: assert(fHits.size()==fCand.getNHits());
228: assert(fHits.size()>1);

Genfit/GFTrack.h
27:#include"assert.h"
294: assert(irep<fBookkeeping.size());
337: assert((unsigned int)index<getNumReps());
389: assert(irep<getNumReps());
396: assert(irep<getNumReps());

Genfit/GFTrackCand.cxx
33: assert(fDetId.size()==fHitId.size());
39: assert(fDetId.size()==fHitId.size());
40: assert(fDetId.size()==fRho.size());
97: assert(fDetId.size()==fHitId.size());

Genfit/GFTrackCand.h
27:#include "assert.h"
86: assert(i<getNHits());
96: assert(i<getNHits());
107: assert(i<getNHits());

Genfit/GFWireHitPolicy.cxx
25:#include "assert.h"
72: assert(rC.GetNrows()==7);
95: assert(x.GetNrows()==7);

Genfit/GFWirepointHitPolicy.cxx
25:#include "assert.h"
76: assert(rC.GetNrows()==8);
99: assert(x.GetNrows()==8);

Genfit/PointHit.cxx
45: //assert (res.size()==4);

Genfit/RKTrackRep.cxx
30:#include "assert.h"
1166: //assert(fabs(checkSum-coveredDistance)<1.E-7);

LArG4/MuNuclearSplittingProcess.cxx
49: assert (0 != particleChange);

LArG4/MuNuclearSplittingProcessXSecBias.cxx
111: assert (0 != particleChange);

LArG4/OpDetReadoutGeometry.cxx
116: assert(0);

LArG4/OpParamAction.cxx
9:#include <cassert>
109: assert(0);
116: assert(0);

LArG4/OpParamSD.cxx
51: assert(0);

LArPandoraAlgorithms/LArClustering/ClusterCreationAlgorithm.cc
13:#include <cassert>

RawData/AuxDetDigit.cxx
14:#include <cassert>

RawData/OpDetPulse.cxx
18:#include <cassert>

RawData/RawDigit.cxx
13:#include <cassert>

RecoAlg/KalmanFilterAlg.cxx
201: assert(prop != 0);
254: assert(dir == Propagator::BACKWARD);
272: assert(gr.getPlane() >= 0);
294: assert(dir Propagator::BACKWARD);
347: assert(dir Propagator::BACKWARD);
431: assert(dir == Propagator::BACKWARD);
487: assert(prop != 0);
552: assert(dir Propagator::FORWARD || dir Propagator::BACKWARD);
553: assert(trk != 0);
580: assert(dir Propagator::BACKWARD);
636: assert(dir Propagator::BACKWARD);
709: assert(dir Propagator::BACKWARD);
743: assert(dir Propagator::BACKWARD);
793: assert(prop != 0);
900: assert(dir == Propagator::BACKWARD);
918: assert(gr.getPlane() >= 0);
940: assert(dir Propagator::BACKWARD);
993: assert(dir Propagator::BACKWARD);
1068: assert(dir == Propagator::BACKWARD);
1113: assert(trh.getStat() != KFitTrack::INVALID);
1518: assert(trh.getStat() != KFitTrack::INVALID);
1708: assert(nb >= 1);
1709: assert(ne >= 1);
1713: assert(sep >= 0.);

RecoAlg/SpacePointAlg.cxx
246: assert(hits.size() 3);
427: assert(hitWireID.TPC tpc && hitWireID.Cryostat cstat);
478: assert(fSptHitMap.count(sptid) 0);
698: assert(fSptHitMap.count(sptid) 0);
713: //\todo change asserts to throwing cet::exceptions
714: assert(hitWireID.Cryostat cstat0);
715: assert(hitWireID.TPC tpc0);
716: assert(hitWireID.Plane < nplanes);
981: assert(mcinfo.xyz.size() 3);
994: assert(mcinfo2.xyz.size() == 3);
1098: assert(hitmap[cstat][tpc][plane1].size() <= hitmap[cstat][tpc][plane2].size());
1114: assert(phit1WireID.Cryostat cstat);
1115: assert(phit1WireID.TPC tpc);
1116: assert(phit1WireID.Plane plane1);
1242: assert(phit1WireID.Cryostat cstat);
1243: assert(phit1WireID.TPC tpc);
1244: assert(phit1WireID.Plane plane1);
1245: assert(phit1WireID.Wire == wire1);
1392: assert(best_spt != 0);

RecoObjects/KGTrack.cxx
168: assert(p != 0.);
185: assert(!!dist);
212: assert(view >= 0 && view < nview);

RecoObjects/KHitContainer.cxx
11:#include <cassert>
52: assert(prop != 0);
118: assert(plane >= 0 && plane < int(planehits.size()));

RecoObjects/KHitContainerWireX.cxx
82: assert(pgr != 0);

RecoObjects/KHitGroup.cxx
52: assert(hit->getMeasPlane() >= 0);
62: assert(fPlane >= 0);

RecoObjects/Propagator.cxx
138: assert(smax > 0.);
304: assert(trk.getSurface()->isEqual(*(ref->getSurface())));

RecoObjects/PropXYZPlane.cxx
11:#include <cassert>
151: assert(dir1 Surface::FORWARD || dir1 Surface::BACKWARD);
225: assert(dir2 Surface::FORWARD || dir2 Surface::BACKWARD);

RecoObjects/PropYZPlane.cxx
11:#include <cassert>
105: assert(dir1 Surface::FORWARD || dir1 Surface::BACKWARD);
149: assert(dir2 Surface::FORWARD || dir2 Surface::BACKWARD);

RecoObjects/test/KalmanFilterTest_module.cc
9:#include <cassert>
49: // Make sure assert is enabled.
51: bool assert_flag = false;
52: assert((assert_flag = true, assert_flag));
53: if ( ! assert_flag ) {
96: assert(ok);

RecoObjects/test/LATest.cxx
8:#include <cassert>
15: // Make sure assert is enabled.
17: bool assert_flag = false;
18: assert((assert_flag = true, assert_flag));
19: if ( ! assert_flag ) {
35: assert(ok);
43: assert(std::abs(unit1(i,j) - val) < 1.e-10);
56: assert(ok);
64: assert(std::abs(unit2(i,j) - val) < 1.e-10);
80: assert(ok);
88: assert(std::abs(unit3(i,j) - val) < 1.e-10);
104: assert(ok);
112: assert(std::abs(unit4(i,j) - val) < 1.e-10);
128: assert(ok);
136: assert(std::abs(unit5(i,j) - val) < 1.e-10);

RecoObjects/test/PropTest_module.cc
9:#include <cassert>
50: // Make sure assert is enabled.
52: bool assert_flag = false;
53: assert((assert_flag = true, assert_flag));
54: if ( ! assert_flag ) {
158: assert(false);
184: assert(pdotdx * (*dist12) > 0.);
190: assert(std::abs(dist - std::abs(*dist12)) <= 1.e-10);
198: assert(std::abs(m0) <= 1.e-8);
199: assert(std::abs(m1) <= 1.e-8);
200: assert(std::abs(m2) <= 1.e-8);
208: assert(std::abs(d0) <= 1.e-8);
209: assert(std::abs(d1) <= 1.e-8);
210: assert(std::abs(d2) <= 1.e-8);
218: assert(!!stat);
234: assert(!!stata);
243: assert(!!statb);
249: assert(std::abs(dij - pm(i,j)) <= 1.e-5*std::abs(dij));
257: assert(!!dist21);
258: assert(std::abs(dist - std::abs(*dist21)) <= 1.e-10);
268: assert(std::abs(vec1(i) - vec2(i)) <= 1.e-8);
270: assert(std::abs(err1(i,j) - err2(i,j)) <= 1.e-8);
331: assert(std::abs(vec1(i) - vec2(i)) < 1.e-6);
333: assert(std::abs(err1(i,j) - err2(i,j)) <= 1.e-6);
439: assert(false);
465: assert(pdotdx * (*dist12) > 0.);
471: assert(std::abs(dist - std::abs(*dist12)) <= 1.e-10);
479: assert(std::abs(m0) <= 1.e-8);
480: assert(std::abs(m1) <= 1.e-8);
481: assert(std::abs(m2) <= 1.e-8);
489: assert(std::abs(d0) <= 1.e-8);
490: assert(std::abs(d1) <= 1.e-8);
491: assert(std::abs(d2) <= 1.e-8);
499: assert(!!stat);
515: assert(!!stata);
524: assert(!!statb);
530: assert(std::abs(dij - pm(i,j)) <= 1.e-5*std::abs(dij));
538: assert(!!dist21);
539: assert(std::abs(dist - std::abs(*dist21)) <= 1.e-10);
549: assert(std::abs(vec1(i) - vec2(i)) <= 1.e-6);
551: assert(std::abs(err1(i,j) - err2(i,j)) <= 1.e-5);
610: assert(std::abs(vec1(i) - vec2(i)) < 1.e-6);
612: assert(std::abs(err1(i,j) - err2(i,j)) <= 1.e-6);

RecoObjects/test/SurfaceTest.cxx
9:#include <cassert>
18: // Make sure assert is enabled.
20: bool assert_flag = false;
21: assert((assert_flag = true, assert_flag));
22: if ( ! assert_flag ) {
36: assert(surf1.isEqual(surf2));
37: assert(!surf1.isEqual(surf3));
38: assert(!surf1.isEqual(surf4));
39: assert(!surf2.isEqual(surf3));
40: assert(!surf2.isEqual(surf4));
41: assert(!surf3.isEqual(surf4));
46: assert(surf1.isParallel(surf2));
47: assert(surf1.isParallel(surf3));
48: assert(!surf1.isParallel(surf4));
49: assert(surf2.isParallel(surf3));
50: assert(!surf2.isParallel(surf4));
51: assert(!surf3.isParallel(surf4));
62: assert(std::abs(xyz1[i] - xyz2[i]) < 1.e-6);
67: assert(surf1.distanceTo(surf2) 0.);
68: assert(surf1.distanceTo(surf3) 1.);
69: assert(surf3.distanceTo(surf1) -1.);
80: assert(caught);
100: assert(std::abs(xyz[0] - 0.1) < 1.e-6);
101: assert(std::abs(xyz[1] - 0.2) < 1.e-6);
102: assert(std::abs(xyz[2]) < 1.e-6);
104: assert(std::abs(xyz[0] - 0.1) < 1.e-6);
105: assert(std::abs(xyz[1] - 1.2) < 1.e-6);
106: assert(std::abs(xyz[2] - 1.0) < 1.e-6);
108: assert(std::abs(mom[0] - 4./std::sqrt(14.)) < 1.e-6);
109: assert(std::abs(mom[1] - 6./std::sqrt(14.)) < 1.e-6);
110: assert(std::abs(mom[2] - 2./std::sqrt(14.)) < 1.e-6);
112: assert(std::abs(mom[0] + 4./std::sqrt(14.)) < 1.e-6);
113: assert(std::abs(mom[1] + 6./std::sqrt(14.)) < 1.e-6);
114: assert(std::abs(mom[2] + 2./std::sqrt(14.)) < 1.e-6);
116: assert(std::abs(mom[0] - 4./std::sqrt(14.)) < 1.e-6);
117: assert(std::abs(mom[1] - (6.*std::cos(1.) - 2.*std::sin(1.))/std::sqrt(14.)) < 1.e-6);
118: assert(std::abs(mom[2] - (6.*std::sin(1.) + 2.*std::cos(1.))/std::sqrt(14.)) < 1.e-6);
129: assert(caught);
141: assert(surf1x.isEqual(surf2x));
142: assert(!surf1x.isEqual(surf3x));
143: assert(!surf1x.isEqual(surf4x));
144: assert(!surf2x.isEqual(surf3x));
145: assert(!surf2x.isEqual(surf4x));
146: assert(!surf3x.isEqual(surf4x));
151: assert(surf1x.isParallel(surf2x));
152: assert(surf1x.isParallel(surf3x));
153: assert(!surf1x.isParallel(surf4x));
154: assert(surf2x.isParallel(surf3x));
155: assert(!surf2x.isParallel(surf4x));
156: assert(!surf3x.isParallel(surf4x));
167: assert(std::abs(xyz1[i] - xyz2[i]) < 1.e-6);
172: assert(surf1x.distanceTo(surf2x) 0.);
173: assert(surf1x.distanceTo(surf3x) 1.);
174: assert(surf3x.distanceTo(surf1x) -1.);
185: assert(caught);

RecoObjects/test/TrackTest.cxx
8:#include <cassert>
15: // Make sure assert is enabled.
17: bool assert_flag = false;
18: assert((assert_flag = true, assert_flag));
19: if ( ! assert_flag ) {
32: assert(!trk.isValid());
33: assert(trf.getStat() trkf::KFitTrack::INVALID);

Simulation/ParticleList.cxx
23:#include &lt;cassert&gt;
281: assert(kFALSE);

TrackFinder/SeedAna_module.cc
141: assert(nbins hden->GetNbinsX());
142: assert(nbins heff->GetNbinsX());
175: assert(nbins hden->GetNbinsX());
176: assert(nbins == hmul->GetNbinsX());
671: assert(part != 0);

TrackFinder/SpacePointAna_module.cc
287: //assert(geo_view hit_view);
291: //assert(channel hit.Channel());

TrackFinder/SpacePointCheater_module.cc
203: assert(hits.size() >= nihits);
207: assert(hits.size() == nihits);
273: assert(hits.size() >= nihits + njhits);
277: assert(hits.size() == nihits + njhits);

TrackFinder/SpacePointFinder_module.cc
203: assert(hits.size() >= nihits);
207: assert(hits.size() == nihits);
273: assert(hits.size() >= nihits + njhits);
277: assert(hits.size() nihits + njhits);

TrackFinder/TrackAna_module.cc
132: assert(nbins hden->GetNbinsX());
133: assert(nbins == heff->GetNbinsX());
653: assert(part != 0);
765: //assert (nsppts_assn == nsppts);
873: assert(part != 0);
877: assert(fMCHistMap.count(pdg) > 0);

TrackFinder/TrackKalmanCheater_module.cc
297: assert(part != 0);
335: assert(trackhits.size() > 0);
363: assert(plane >= 0 && plane < planehits.size());

Utilities/FileCatalogMetadataExtras_service.cc
168: assert(fPerFileMetadataMap.count(fn) != 0);

Utilities/SignalShaping.cxx
11:#include <cassert>
87: assert(kern.size() fConvKernel.size());
202: assert(fResponse.size() n);
203: assert(2 * (fConvKernel.size() - 1) n);
240: assert(2 * (fFilter.size() - 1) n);
241: assert(fFilter.size() == fConvKernel.size());
272: assert(peak_response > 0); // Peak should always be positive.
282: assert(peak_deconv > 0); // Peak should always be positive.

Utilities/test/LArPropTest_module.cc
43: // Make sure assert is enabled.
45: bool assert_flag = false;
46: assert((assert_flag = true, assert_flag));
47: if ( ! assert_flag ) {
69: assert(larprop->Density() == larprop->Density(larprop->Temperature()));
70: assert(larprop->Density() != larprop->Density(larprop->Temperature()+0.1));
71: assert(larprop->DriftVelocity() larprop->DriftVelocity(larprop->Efield()));
72: assert(larprop->DriftVelocity() larprop->DriftVelocity(larprop->Efield(),

Utilities/test/SignalShapingLBNE10ktTest_module.cc
32: assert(h != 0);
53: assert(d >= 0 && d < n);

Utilities/test/SignalShapingLBNE35tTest_module.cc
32: assert(h != 0);
53: assert(d >= 0 && d < n);

Utilities/test/SignalShapingMicroBooNETest_module.cc
32: assert(h != 0);
53: assert(d >= 0 && d < n);

AssertionsInCode.txt (18.2 KB) AssertionsInCode.txt Gianluca Petrillo, 04/09/2014 07:19 PM

History

#1 Updated by Erica Snider almost 7 years ago

  • Status changed from New to Assigned
  • Assignee set to Erica Snider
  • Target version set to v1_01_00

We will not be able to solve this before the production transition, so may need to enable asserts in until the policy is clarified.

#2 Updated by Gianluca Petrillo over 6 years ago

  • Assignee changed from Erica Snider to Gianluca Petrillo

#3 Updated by Gianluca Petrillo over 6 years ago

  • % Done changed from 0 to 90

I have checked the assertions one by one.
I have turned most of them in exceptions, and left a few where the check was a sanity check or where turning into an exception (and having the check performed every turn) would risk to affect the performances.
Assertions in test directories have been left untouched.
I have tested the code with prodsingle.fcl, prodsingle_uboone.fcl, and a few events with a 5-step chain of MicroBooNE beam+cosmic production.

Assertions are still enabled in CMakeLists.txt, but everything should be ready to disable them for the optimized build.

#4 Updated by Gianluca Petrillo over 6 years ago

  • Status changed from Assigned to Feedback

If the changes are acceptable, the last step is to remove cet_enable_asserts() from the CMakeLists.txt files (except the test ones).
This might be considered a breaking change, so we might want to follow the relative protocol.

#5 Updated by Gianluca Petrillo over 6 years ago

Attaching the list of changes

#6 Updated by Gianluca Petrillo over 6 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 90 to 100

cet_enable_asserts macro removed from all the non-test CMakeFiles.txt. art/CET policies are now followed by default.
LOG_DEBUG macro does produce (almost) no code except in debug builds.
assert macro have no effect except in debug builds.
mf::LogDebug output is still processed and follows the current configuration of messagefacility, regardless the type of build.
Connected to issues #5147 and #5589.

#7 Updated by Gianluca Petrillo over 6 years ago

  • Status changed from Resolved to Assigned
  • Priority changed from Normal to Immediate

Unfortunately disabling asserts awoke some spurious compiler warnings, which we are taking as errors.
Also some test CMakeLists.txt files are missing cet_enable_asserts macro.
Fix soon.

#8 Updated by Gianluca Petrillo over 6 years ago

  • Status changed from Assigned to Resolved

Added missing cet_enable_asserts() to the test CMakeFiles.txt with the idea that they do not hurt.
Also converted an assertion which was left back into an exception and some workarounds for spurious "may be used uninitialized in this function" warning from gcc pointing to boost.

#9 Updated by Gianluca Petrillo over 6 years ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF