Range check issue in CTPPS breaking IB

[francescobrivio]

There is a(nother) crash observed in the last IB (spotted by @mmusich) which seems also related to CTPPS:
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_2_X_2021-10-31-2300/pyRelValMatrixLogs/run/136.8642_RunJetHT2018BHEfail+RunJetHT2018BHEfail+HLTDR2_2018+RECODR2_2018reHLT_skimJetHT_Prompt_HEfail+HARVEST2018_HEfail/step3_RunJetHT2018BHEfail+RunJetHT2018BHEfail+HLTDR2_2018+RECODR2_2018reHLT_skimJetHT_Prompt_HEfail+HARVEST2018_HEfail.log#/833

----- Begin Fatal Exception 01-Nov-2021 03:11:13 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Processing  Event run: 317435 lumi: 36 event: 47465289 stream: 3
   [1] Running path 'dqmoffline_step'
   [2] Prefetching for module DQMMessageLogger/'DQMMessageLogger'
   [3] Prefetching for module LogErrorHarvester/'logErrorHarvester'
   [4] Calling method for module CTPPSProtonProducer/'ctppsProtons'
Exception Message:
A std::exception was thrown.
vector::_M_range_check: __n (which is 0) >= this->size() (which is 0)
----- End Fatal Exception -------------------------------------------------

It looks like an empty std::vector evaluated at(0).

[cmsbuild]

A new Issue was created by @francescobrivio .

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

cms-bot commands are listed here

[francescobrivio]

Not sure who should be assigned here.
FYI @cms-sw/ctpps-dpg-l2

[VinInn]

good that the issue is cough. Still production code should not use .at. Please use operator[]

[jan-kaspar]

good that the issue is cough. Still production code should not use .at. Please use operator[]

We can switch to []. For my education, why should it be preferred over at?

[mmusich]

much faster

[jan-kaspar]

much faster

Thanks!

[jan-kaspar]

I understand that the operator[] preference holds for std::vector. Does the same apply for std::map? For map, there is a complication that operator[] is not const, while at() can be - thus useful for const getters e.g. here:
https://github.com/cms-sw/cmssw/blob/master/CondFormats/PPSObjects/interface/PPSAssociationCuts.h#L73

[makortel]

I understand that the operator[] preference holds for std::vector.

Correct. For operator[]() the validity of the index should be checked elsewhere or come from other guarantees. (i.e. it is best to organize code such that such guarantees can be cheaply given).

. Does the same apply for std::map? For map, there is a complication that operator[] is not const, while at() can be

No, as you pointed out. In std::map the difference between operator[]() and at() is that in case the key does not exist yet, [] adds a new element, whereas at() throws an exception (and if one needs to act differently than terminating the program when key is missing, one should use find()).

[VinInn]

at shall not be used even in case of map we do not want exception to be thrown.
find shall be used

[VinInn]

BTW: map should be avoided in reco code. There is essentially no use case for that.

[jan-kaspar]

Hopefully, here's a fix: #35941.

[tvami]

assign alca

[cmsbuild]

New categories assigned: alca

@yuanchao,@francescobrivio,@malbouis,@tvami you have been requested to review this Pull request/Issue and eventually sign? Thanks

[francescobrivio]

+alca

• IBs clean now see CTPPS related issue in several IB workflows #35928 (comment)

[cmsbuild]

This issue is fully signed and ready to be closed.