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

[RFC] Call nanoAOD_runMETfixEE2017() only if the modifiers are chosen #287

Merged

Conversation

makortel
Copy link

Originally from cms-sw#25587

The customizations

for modifier in run2_nanoAOD_94XMiniAODv1, run2_nanoAOD_94XMiniAODv2:
modifier.toModify(process, nanoAOD_runMETfixEE2017(process,isData=True))

and
for modifier in run2_nanoAOD_94XMiniAODv1, run2_nanoAOD_94XMiniAODv2:
modifier.toModify(process, nanoAOD_runMETfixEE2017(process,isData=False))

were introduced in cms-sw#25373. The code look like the intention was to call nanoAOD_runMETfixEE2017() only if run2_nanoAOD_94XMiniAODv1 or run2_nanoAOD_94XMiniAODv2 modifiers are turned on.

The actual behavior is, however, different. The lines 276 and 283 contain a call to nanoAOD_runMETfixEE2017() function (instead of a function "object" to be called by toModify()), so it is called regardless of the status of the two modifiers (and twice because of the loop). The function itself

def nanoAOD_runMETfixEE2017(process,isData):
runMetCorAndUncFromMiniAOD(process,isData=isData,
fixEE2017 = True,
fixEE2017Params = {'userawPt': True, 'ptThreshold':50.0, 'minEtaThreshold':2.65, 'maxEtaThreshold': 3.139},
postfix = "FixEE2017")
process.nanoSequenceCommon.insert(process.nanoSequenceCommon.index(jetSequence),process.fullPatMetSequenceFixEE2017)

does not explicitly return anything, so the return value is None. The Modifier then interprets that
def toModify(self,obj, func=None,**kw):

such that the func argument is None, and there are no keyword arguments, so it has nothing to do.

If my guess above on the intention is correct, this PR suggests to fix the behavior to the intended one.

Tested in 10_4_0.

@gpetruc-bot
Copy link

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 672679

Code integration

Code checks not run for this PR (no source files modified)

Tests

  • Long test data101X (10000 events): passed, no significant changes; dqm plots: all, diff
  • 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 data94X2016 (10000 events): passed, no significant changes; dqm plots: all, diff
  • Long test data94Xv2 (10000 events): passed, no significant changes; dqm plots: all, diff
  • Long test mc102X (9000 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
  • Long test mc94X2016 (9000 events): passed, no significant changes; dqm plots: all, diff
  • Long test mc94Xv2 (9000 events): passed, no significant changes; dqm plots: all, diff
  • Test mc_94Xv2: passed
  • Test mc_102X: passed
  • Test data_94X: passed
  • Test data_101X: passed

Disk size report

Sample kb/event ref kb/event diff
TTbar MC 94Xv1 1.656 1.658 -0.001 ( -0.1% )
TTbar MC 94Xv2 1.669 1.670 -0.001 ( -0.1% )
TTbar MC 94X2016 1.528 1.528 -0.000 ( -0.0% )
TTbar MC 80X 1.686 1.686 -0.001 ( -0.0% )
Data 94Xv1 0.723 0.723 0.000 ( +0.0% )
Data 80X 0.626 0.625 0.000 ( +0.0% )
Data 80X, Mu Run2016E 0.612 0.612 -0.000 ( -0.0% )

@peruzzim peruzzim merged commit 1b19348 into cms-nanoAOD:master-cmsswmaster Jan 24, 2019
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

3 participants