cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.08k stars 4.3k forks source link

TauWPThreshold uses exceptions in string parsing #39627

Open makortel opened 2 years ago

makortel commented 2 years ago

The deep_tau::TauWPThreshold constructor uses exceptions string parsing https://github.com/cms-sw/cmssw/blob/bae128ce493657abe49691256ff76a9a63a84c3a/RecoTauTag/RecoTau/src/DeepTauBase.cc#L19-L25

I encountered this exception being thrown many times (don't know how many because I gave up) when trying to catch a different exception, so the parse error does not seem to be as exceptional that exceptions would be justified (see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideEdmExceptionUse#Exception_Handling, https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Re-throw https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Re-errors)

I'd suggest to parse the string in a way that does not use exceptions to signal an invalid value. E,g, std::from_chars() looks like a good candidate, but GCC (or libstdc++) implements it for floating point numbers only in version 11 (to which we'll hopefully switch to soon). An alternative could be std::strtod (from C) or std::istringstream (expensive).

makortel commented 2 years ago

assign reconstruction

cmsbuild commented 2 years ago

New categories assigned: reconstruction

@mandrenguyen,@clacaputo you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild commented 2 years ago

A new Issue was created by @makortel Matti Kortelainen.

@Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

clacaputo commented 2 years ago

type tau

@cms-sw/tau-pog-l2

kandrosov commented 2 years ago

Since parsing occurs only at the initialisation stage, I think std::istringstream is a best option for now. Once std::from_chars is available - we can switch to it.