addressing issue 19340, adding checks to avoid possible acces violations

[alberto-sanchez]

We are trying to address possible access to undefined entries in vector. #19340

[cmsbuild]

A new Pull Request was created by @alberto-sanchez (Alberto Sanchez Hernandez) for master.

It involves the following packages:

HeavyFlavorAnalysis/Onia2MuMu

@cmsbuild, @monttj, @davidlange6 can you please review it and eventually sign? Thanks.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

[slava77]

@alberto-sanchez thank you for the quick update.

The code looks good, visually.

I will not start the cms-bot tests for now because the IB is broken.

[alberto-sanchez]

@slava77 I run 136.761 and was unable to reproduce the crash. I also have compare the outcome in my MC sample for chiB, and found no differences. I assume, the events are producing the crash are very special ones.

[slava77]

On 6/21/17 2:01 PM, Alberto Sanchez Hernandez wrote: @slava77 <https://github.com/slava77> I run 136.761 and was unable to reproduce the crash. I also have compare the outcome in my MC sample for chiB, and found no differences. I assume, the events are producing the crash are very special ones.

There was actually no report of a crash (yet), only a report of invalid reads from outside the vector bounds.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19401 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbj6wCm1VraUXPttFR1ET8RlixTn-ks5sGYTEgaJpZM4OBb2q>.

[alberto-sanchez]

if cms-bot is back, can we trigger the tests?

[slava77]

@cmsbuild please test

the IB was available since ~11:30 thursday

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20849/console Started: 2017/06/22 17:12
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20855/console Started: 2017/06/22 19:06

[cmsbuild]

+1
Tested at: 0e402c1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19401/20855/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
e0be6a0
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19401/20855/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19401/20855/git-merge-result

[cmsbuild]

Comparison job queued.

[slava77]

@smuzaffar

it looks like the logic to detect the recent commits is off.
The parent commit of this PR is 008015a which earlier or the same as CMSSW_9_2_X_2017-06-21-2300.

The PR tests in #19401 (comment) were made in CMSSW_9_2_X_2017-06-22-1100

and the bot is claiming that

The following merge commits were also included 
on top of IB + this PR after doing git cms-merge-topic:
e0be6a0

where this commit mentioned is the same as CMSSW_9_2_X_2017-06-22-1100

this comes from
+ git log '--after=2017-06-22 09:50:06 +0200' HEAD --merges
while the commit e0be6a0 in question is Date: Thu Jun 22 09:50:06 2017 +0200
and should not be listed as an additional merge on top of itself

Please check.
Thank you.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19401/20855/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 22
• DQMHistoTests: Total histograms compared: 1802882
• DQMHistoTests: Total failures: 29330
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 1773386
• DQMHistoTests: Total skipped: 166
• DQMHistoTests: Total Missing objects: 0
• Checked 90 log files, 14 edm output root files, 22 DQM output files

[davidlange6]

hi @alberto-sanchez - lets simply this

check at the start - that there are 2 tracks from the conversion
then loop from i=0; i<2; i++.. and do the work that is there. Then lots of this logic can just be removed..

[alberto-sanchez]

@davidlange6 sorry for my ignorance, I do not see how this the will simplify the logic. At the end,
I still need to compare (if exists) the closest PVs between the two tracks. So, that last part has to be
the same.

[davidlange6]

yes, i misunderstood the logic.

[davidlange6]

merge