Streamline MuonRawToCoord
- do not use Error(...) but instead throw exception
- put all code in .cpp file
- push some code into freestanding function instead of member function
this should simplify the migration to Gaudi::Algorithm from GaudiAlgorithm
Merge request reports
Activity
I created a merge request to LHCb (!1721 (merged)) and to Rec (!1398 (merged)) that include new tools to access the geometry. This reduces by a factor 4 the timing needed for the decoding. They also remove the intermediate step of creating the MuonCoords before passing to MuonHits.
Still this is under development
Looks like this MR and !1721 (merged) do not overlap, so I propose to just move this forward -- in addition, in this MR some (more) code is pushed into freestanding functions, which (eventually) might be shared with the code in !1721 (merged)...
- [2019-02-02 00:04] Validation started with lhcb-lcg-dev4#797
- [2019-02-03 00:03] Validation started with lhcb-head#2135
- [2019-02-03 00:04] Validation started with lhcb-tdr-test#443
- [2019-02-03 00:05] Validation started with lhcb-gaudi-head#2149
- [2019-02-03 00:05] Validation started with lhcb-lcg-dev4#798
- [2019-02-03 00:06] Validation started with lhcb-dd4hep#140
- [2019-02-03 00:07] Validation started with lhcb-lcg-dev3#792
- [2019-02-03 00:07] Validation started with lhcb-sanitizers#145
Edited by Software for LHCb
Apart from a new counter, Brunel tests are clean, this seems good to go:
Different set of Counters for algo MuonRawToCoord Ref has 3 Counters, found 4 of them in stdout Counters in stdout and not in ref : ['"#digits - estimated # digits"']https://lhcb-nightlies.cern.ch/logs/tests/nightly/lhcb-lcg-dev4/797/x86_64-centos7-gcc7-opt/Brunel/
correct -- I added a counter to check whether the a-priori estimated # of digits matches the final number. If positive, that implies that the code had to make additional memory allocations... (aside: @sponce: I wanted to make a custom counter set which does max and average, but couldn't quite figure out how to do that)
mentioned in merge request Brunel!637 (merged)
mentioned in commit 65d133a3
mentioned in commit Brunel@549d7040
102 auto preamble_size = 2*((rawdata[0]+3)/2); 103 if (rawdata.size()<preamble_size) OOPS( MuonRaw::ErrorCode::PADDING_TOO_LONG ); 104 rawdata = rawdata.subspan( preamble_size ); 105 for (int i=0;i<4;++i) { 106 if (UNLIKELY( rawdata.empty())) OOPS( MuonRaw::ErrorCode::BANK_TOO_SHORT ); 107 if (UNLIKELY( rawdata.size() < 1 + rawdata[0] )) OOPS( MuonRaw::ErrorCode::TOO_MANY_HITS ); 108 for (unsigned int pp : rawdata.subspan(1,rawdata[0])) { 109 unsigned int add = (pp&0x0FFF); 110 unsigned int tdc_value = ((pp&0xF000)>>12); 111 std::optional<LHCb::MuonTileID> tile = make_tile(add); 112 if (UNLIKELY(!tile.has_value())) continue; 113 *out++ = { std::move(*tile), tdc_value }; 114 } 115 rawdata = rawdata.subspan(1+rawdata[0]); 116 } 117 assert(rawdata.empty()); -
@graven Sorry I did not notice this earlier, but the brunel-upgrade-veloutmuonmatch test is failing in debug mode with the following assert:
python: /workspace/build/LHCB/LHCB_HEAD/Muon/MuonDAQ/src/MuonRawToCoord.cpp:117: OutputIterator {anonymous}::decodeTileAndTDCV1(gsl::span<const short unsigned int, -1>, MakeTile&&, OutputIterator) [with MakeTile = MuonRawToCoord::operator()(const LHCb::RawEvent&) const::<lambda(unsigned int)>; OutputIterator = std::back_insert_iterator<std::vector<{anonymous}::Digit> >]: Assertion `rawdata.empty()' failed.Could you take a look please? Full log at https://lhcb-nightlies.cern.ch/logs/tests/nightly/lhcb-head/2139/x86_64-centos7-gcc7-dbg/Brunel/
