Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed segmentation violation that occurred in subjet analyses in case… #111

Merged
merged 1 commit into from Mar 14, 2018

Conversation

ktht
Copy link

@ktht ktht commented Feb 9, 2018

… a pat::Jet had daughters of type pat::Jet

(previous code only handled the case that daughters of pat::Jet were of type reco::PFJet)

… a pat::Jet had daughters of type pat::Jet

(previous code only handled the case that daughters of pat::Jet were of type reco::PFJet)
@arizzi
Copy link

arizzi commented Feb 13, 2018

can we send this directly to cmssw? master & 94X branches

@veelken
Copy link

veelken commented Feb 21, 2018

I confirm that the fix can go into cmssw (master & 94X branches)

@arizzi
Copy link

arizzi commented Feb 21, 2018

can you guys open the relevant PR in master and 94X branches?

@ktht
Copy link
Author

ktht commented Feb 21, 2018

Which one(s) from the 94X branches exactly? CMSSW_9_4_X? Anything else?

@arizzi
Copy link

arizzi commented Feb 21, 2018

I think this can go into main CMSSW_9_4_X and will be auto fwd ported to MAOD branch

@gpetruc
Copy link

gpetruc commented Feb 22, 2018

I think we can merge it also in here (it's the same commit as cms-sw#22294 ) if it doesn't trigger a too lengthy recompilation - testing that now using the bot

@gpetruc-bot
Copy link

@veelken
Copy link

veelken commented Feb 22, 2018

The corresponding PR for the CMSSW 9_4_x branch is here: cms-sw/cmssw@CMSSW_9_4_X...HEP-KBFI:fixSIGSEGV

Copy link

@gpetruc-bot gpetruc-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automatic test report for 312110

Code integration

Code checks passed for this PR

Tests

  • Long test data80X (10000 events): passed, no significant changes; dqm plots: all, diff
  • Long test data80Xhip (3000 events): passed, no significant changes; dqm plots: all, diff
  • Long test data94X (10000 events): passed, no significant changes; dqm plots: all, diff
  • Long test mc80X (10000 events): passed, no significant changes; dqm plots: all, diff
  • Long test mc94X (10000 events): passed, no significant changes; dqm plots: all, diff
  • Test mc_94X: passed
  • Test mc_80X: passed
  • Test mc_92X: passed
  • Test data_92X: passed
  • Test data_80X: passed

Disk size report

Sample kb/event ref kb/event diff
TTbar MC 94X 1.509 1.509 0.000 ( +0.0% )
TTbar MC 80X 1.557 1.557 -0.000 ( -0.0% )
Data 94X 0.611 0.610 0.000 ( +0.0% )
Data 80X 0.577 0.577 -0.000 ( -0.0% )
Data 80X, Mu Run2016E 0.508 0.507 0.000 ( +0.1% )

@ktht
Copy link
Author

ktht commented Mar 1, 2018

PR merged in master: cms-sw#22298; 94X still open, though.

@veelken
Copy link

veelken commented Mar 2, 2018

Hi,

would be great if this PR can be merged soon.

Cheers,

Christian

@arizzi
Copy link

arizzi commented Mar 14, 2018

@gpetruc given that there isn't yet a full 94X release with the dependency PR included, how do we want to proceed?

  • wait for 9_4_5
  • merge the needed commit here too? (but then it conflicts on 10XY?)
  • move our ref to an IB

@gpetruc
Copy link

gpetruc commented Mar 14, 2018 via email

@arizzi
Copy link

arizzi commented Mar 14, 2018

but then the fwd port of our features to our 10XY master is less trivial (we have to remember the commit to remove) ...

@arizzi arizzi merged commit 2aa9689 into cms-nanoAOD:master Mar 14, 2018
@ktht ktht deleted the fixSIGSEGV branch April 8, 2018 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants