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

addition of BadPFMuonFilter_Dz_cfi.py MET filter in CMSSW #30015

Merged
merged 14 commits into from Jul 28, 2020

Conversation

amkalsi
Copy link
Contributor

@amkalsi amkalsi commented May 28, 2020

PR description:

Changes made in code to introduce new MET filter

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

Before submitting your pull requests, make sure you followed this checklist:

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30015/15707

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@slava77
Copy link
Contributor

slava77 commented May 28, 2020

Changes made in code to introduce new MET filter

based on the current pre-clang-format changes, it seems that this PR is made from an old (10_6_X?) version of this code.
Please apply your edits directly on top of an 11_1_X version so that we do not have re-making of clang-formatting in the history.

Also, please use a more descriptive title for this PR

@amkalsi
Copy link
Contributor Author

amkalsi commented May 30, 2020

Hi @slava77 , I started from CMSSW_11_1_0_pre8 version and then added the modified files. Could you please let me know what is causing this?
About Code formatting, I ran scram build code-format command and can push the changes in repository

@amkalsi amkalsi mentioned this pull request May 30, 2020
@slava77
Copy link
Contributor

slava77 commented May 30, 2020

Hi @slava77 , I started from CMSSW_11_1_0_pre8 version and then added the modified files. Could you please let me know what is causing this?

Thank you for clarifying.
Looking at 88b2430
it appears like your editor converts spaces to tabs, which all have to be reverted according to #30015 (comment)
Please check if you can disable it so that every next time you edit the file we don't get a two-step process of adding and removing the tabs

About Code formatting, I ran scram build code-format command and can push the changes in repository

To align with my earlier comment, it would be nice to squash the commits to .cc files into one commit.

@slava77
Copy link
Contributor

slava77 commented Jun 4, 2020

ping
@amkalsi please clarify on the status

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

The code-checks are being triggered in jenkins.

@amkalsi
Copy link
Contributor Author

amkalsi commented Jun 4, 2020

Hi @slava77, I submitted the changes with correct format. Please check. Apologies for delay.. had some problems with charger of my laptop

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30015/15873

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

A new Pull Request was created by @amkalsi (Amandeep Kaur Kalsi) for master.

It involves the following packages:

RecoMET/METFilters

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @gouskos, @ahinzmann, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @schoef, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Jun 4, 2020

@cmsbuild please test

@slava77
Copy link
Contributor

slava77 commented Jun 4, 2020

Also, please use a more descriptive title for this PR

@slava77
Copy link
Contributor

slava77 commented Jul 27, 2020

+1

for #30015 bb38311

  • code changes are in line with the PR description and the follow up review
  • jenkins tests pass and comparisons with the baseline show differences only in TriggerResults variables corresponding to addition of a new filter path in the MET filter paths

@silviodonato silviodonato changed the title addition of BadPFMuonFilter_Dz_cfi.py met filter in CMSSW addition of BadPFMuonFilter_Dz_cfi.py MET filter in CMSSW Jul 28, 2020
@silviodonato
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 37a05c6 into cms-sw:master Jul 28, 2020
@Dr15Jones
Copy link
Contributor

This appears to be causing failures in the IB for workflows 136.7722 and 1329.1 with the message

An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 283877 lumi: 11 event: 18022631 stream: 0
   [1] Running path 'nanoAOD_step'
   [2] Calling method for module BadParticleFilter/'BadPFMuonTagger'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vector<reco::Vertex>
Looking for module label: offlinePrimaryVertices
Looking for productInstanceName: 

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------

@slava77
Copy link
Contributor

slava77 commented Jul 29, 2020

@amkalsi
please take a look at the case with errors.
Please clarify if you can work on this today/tomorrow.
Thank you.

@amkalsi
Copy link
Contributor Author

amkalsi commented Jul 29, 2020

Hi @slava77, yes I will look into this. The error seems to be related to nanoAOD step. Could you please confirm? I am not sure how to reproduce this error as PR passed all the basic sets (*). Could you please let me know how did you get this error specifically running which step?
Also, we want to add this flag in nanoAOD samples also. I am trying to figure out how to do that.

(*)
runTheMatrix.py -l limited -i all --ibeos

@slava77
Copy link
Contributor

slava77 commented Jul 29, 2020

workflows 136.7722 and 1329.1 have problems
runTheMatrix.py -l 136.7722,1329.1 -i all --ibeos to reproduce

@amkalsi
Copy link
Contributor Author

amkalsi commented Jul 29, 2020

Hi @slava77, thanks I just made the required change in file : PhysicsTools/NanoAOD/python/extraflags_cff.py
It should work now.

@slava77
Copy link
Contributor

slava77 commented Jul 29, 2020

Hi @slava77, thanks I just made the required change in file : PhysicsTools/NanoAOD/python/extraflags_cff.py
It should work now.

I'm guessing that the change is in master...amkalsi:11_1_X-MET0filter
Please make a new PR with this.

thank you for the quick follow up.

@amkalsi
Copy link
Contributor Author

amkalsi commented Jul 30, 2020

@slava77
Copy link
Contributor

slava77 commented Aug 4, 2020

@amkalsi

from #30015 (comment)
I expect a backport of this for 10_6_X.
Please clarify the status of making the backport.
Thank you.

@amkalsi
Copy link
Contributor Author

amkalsi commented Aug 5, 2020

Hi @slava77 , I have started with CMSSW_10_6_3. Now running tests. I will make PR soon.

@santocch
Copy link

santocch commented Aug 7, 2020

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2020

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants