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

Implementing dedicated mttbar LHE filter (backport of 33541) #33604

Closed

Conversation

gsorrentino18
Copy link
Contributor

PR description:

This PR implements a new filter, requested by GEN conveners, which removes events with invariant TTbar mass below or above given thresholds at LHE level.
An additional cut on the quarks pT is also applied, and both mass and pT thresholds can be customized in the _cfi file.

PR validation:

PR tested in the CMSSW_11_0_3 release.

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

Backport of #33541, needed for TT production within the CMSSW_11_0_3 release

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2021

A new Pull Request was created by @gsorrentino18 (Giulia Sorrentino) for CMSSW_11_0_X.

It involves the following packages:

GeneratorInterface/GenFilters

@SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please review it and eventually sign? Thanks.
@alberto-sanchez, @mkirsano this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@agrohsje
Copy link

agrohsje commented May 3, 2021

Hi @gsorrentino18 this is for Run3WinterGS, right?

@agrohsje
Copy link

agrohsje commented May 3, 2021

please test

@gsorrentino18
Copy link
Contributor Author

Hi @gsorrentino18 this is for Run3WinterGS, right?

Hi Alexander, actually this is for Run3Winter20wmLHEGS

@agrohsje
Copy link

agrohsje commented May 3, 2021

:-D indeed. otherwise a LHE filter is useless.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2021

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79d3e1/14789/summary.html
COMMIT: 2632d94
CMSSW: CMSSW_12_0_X_2021-05-02-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33604/14789/install.sh to create a dev area with all the needed externals and cmssw changes.

This pull request cannot be automatically merged, could you please rebase it?
You can see the log for git cms-merge-topic here: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79d3e1/14789/git-merge-result

@fabiocos
Copy link
Contributor

fabiocos commented May 3, 2021

please test

@fabiocos
Copy link
Contributor

fabiocos commented May 3, 2021

@agrohsje @smuzaffar @silviodonato @qliphy why was the 12_0_X branch picked here for the test? The merge seems to work ok to me

  510  cmsrel CMSSW_11_0_3               
  511  cd CMSSW_11_0_3/src/
  512  cmsenv
  513  git cms-init --upstream-only ; git checkout -b prova ; git cms-merge-topic 33604
  514  git log --topo-order --graph --pretty=oneline
  515  h
  516  ls
  517  git log --topo-order --graph --pretty=oneline

gives

*   8968f67a34c229662e512ac684c3f0e51c7dc723 (HEAD -> prova) Merged refs/pull/33604/head from repository cms-sw with cms-merge-topic
|\  
| * 2632d9467eb61f9193240d2f34a3f4ba45412000 (pull/33604_backup, cms-sw/refs/pull/33604/head) Initializing parameters in the plugin
| * b90b9c6950451752864e3abf617d69794812af9e Deleting obsolete file and modifying MinInvMass value
| * 00c97416ece566a9317b1e228da62e0626a757f4 code-checks and code-format
| * abca2a05b72a55e7073833f5b5293d9e4c9a80d0 Implementing Alexander's comments
| * dad11be6ca1d1795906e58edbd9cc6bfdb6c0d16 New LHE filter
|/  
*   85ad882a12713f89d81fcf0137a471cd287567ba (tag: CMSSW_11_0_3, from-CMSSW_11_0_3) Merge pull request #29540 from alja/palette

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2021

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79d3e1/14805/summary.html
COMMIT: 2632d94
CMSSW: CMSSW_12_0_X_2021-05-03-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33604/14805/install.sh to create a dev area with all the needed externals and cmssw changes.

This pull request cannot be automatically merged, could you please rebase it?
You can see the log for git cms-merge-topic here: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79d3e1/14805/git-merge-result

@qliphy
Copy link
Contributor

qliphy commented May 4, 2021

@agrohsje @smuzaffar @silviodonato @qliphy why was the 12_0_X branch picked here for the test? The merge seems to work ok to me
adding @mrodozov also

Probably we don't have 11_0_X IB now?

@fabiocos
Copy link
Contributor

fabiocos commented May 4, 2021

@qliphy which sounds a bit strange to me. I may understand we are not producing the IBs, as the branch seems mostly inactive (it is not shown any more in the IB dashboard), but the branch itself should have not disappeared, does it? BTW I see a number of merged PRs apparently not part of any new release version in https://github.com/cms-sw/cmssw/commits/CMSSW_11_0_X , the latest release is of about one year ago, according to the release notes https://cmssdt.cern.ch/SDT/ReleaseNotes/CMSSW_11/CMSSW_11_0_3.html ...

@fabiocos
Copy link
Contributor

fabiocos commented May 4, 2021

*   c71a6e954e637e1de0bccc84a239e291bbbd7f65 (HEAD -> CMSSW_11_0_X, official-cmssw/CMSSW_11_0_X) Merge pull request #30300 from christopheralanwest/alca-fix-hlt-jec-11_0_X
|\  
| * 282d9607dd110702e041eee3a2e61105ffefb87a Fix for incorrect Run 3 HLT JEC
|/  
*   1b74e6b681c1cd0b9f4bfa551211ca358e52a4b8 Merge pull request #30217 from lwang046/Default_NoTDCPack110x
|\  
| * 6a26a572d546f6db60a6be8fe46058ea94317e12 Change HB TDC digi default to non-packing in 11_0_X
* |   2f7a9431bbcb9c1cc8f59c7a8127213663c3d6f5 Merge pull request #30203 from christopheralanwest/alca-fix-l1t-issue29237_11_0_X
|\ \  
| |/  
|/|   
| * 54e2b20c051e8fa26e0b459007ab7a381a9166da Updated L1T tags to address CMSSW issue #29237
|/  
*   3b29003f6255c90f780a408e9fe1017edf68b900 Merge pull request #30131 from christopheralanwest/alca-ul-final-data-gt-11_0_X
|\  
| * 74a77e94d4ca0306b49c8562ff03ff6a734879ca Update offline GT for Run 1 and Run 2 data, including special runs.
|/  
*   6031168ed6b0e1c5cf428c92f2031f09f279a8c1 Merge pull request #29867 from intrepid42/Rivet3ForOldSamples_110X
|\  
| * de0ea3a42e4ab12be97da21c5e54bae16e9ed83f code-format
| * b4dae9edda3acf70183fd529a3231cdd8bfb974a Rivet3 fallback for old samples without GenLumiInfoHeader
* |   85ad882a12713f89d81fcf0137a471cd287567ba (tag: CMSSW_11_0_3, from-CMSSW_11_0_3) Merge pull request #29540 from alja/palette
|\ \  

@silviodonato
Copy link
Contributor

@gsorrentino18 could you clarify why you need to use CMSSW_11_0_3? Is it related to the MTD TDR?

@fabiocos you are right, #30300 #30217 #30203 #30131 #29867 were merged after CMSSW_11_0_3 and then they are not included in any release.
The branch is still there https://github.com/cms-sw/cmssw/tree/CMSSW_11_0_X, but we closed the IB in August 2020, after the build of https://github.com/cms-sw/cmssw/releases/CMSSW_11_1_0 and usuage of 11_1_X in the MWGR#2 (CMSSW_11_1_0_patch2).
#31139.

@fabiocos
Copy link
Contributor

fabiocos commented May 4, 2021

@gsorrentino18 will comment about this PR and why it is needed in this cycle, this has nothing to do with MTD to my knowledge. I am just surprised by the fact that the bot does not pick the correct branch, regardless of the IB being run or not. Perhaps I do not properly recall how this situation is handled by it.

@mrodozov
Copy link
Contributor

mrodozov commented May 4, 2021

I see the record for 11_0 doesn't have a build hour which might explain why there is no IB
I have to go and see what the bot did, because it has to pick the 11_0_X branch looking here:
https://github.com/cms-sw/cms-bot/blob/master/config.map#L73
and there is only one line so it should get gcc820 etc all that is in that one line because there is no other match

@gsorrentino18
Copy link
Contributor Author

@gsorrentino18 could you clarify why you need to use CMSSW_11_0_3? Is it related to the MTD TDR?

Hi Silvio, this PR is not related to MTD.
I need to use this LHE filter in the fragment needed for the production of a TTbar sample requested by EGM (I’m their MC contact) in the Run3Winter20wmLHEGS campaign, which uses the CMSSW_11_0_3 release.

@smuzaffar
Copy link
Contributor

11.0.X IBs have been disabled since July last year
cms-sw/cms-bot@a4383bd#diff-c7a93337cd83c53d1407e8d572a00744b2f5efe9e730bc98477f2bc21150292d

@fabiocos
Copy link
Contributor

fabiocos commented May 4, 2021

@smuzaffar the bot is based on the cmssw-ib repository, right? And as this is not fed the bot picks the main master, I guess...

@mrodozov
Copy link
Contributor

mrodozov commented May 4, 2021

I missed the "disabled=1" (maybe because it wasn't last) :/

@smuzaffar
Copy link
Contributor

@fabiocos , yes I think bot did not find the correct release and then tried to use the master. I will fix bot to properly handle such cases

@silviodonato silviodonato marked this pull request as draft May 4, 2021 10:56
@silviodonato silviodonato marked this pull request as ready for review May 4, 2021 10:58
@silviodonato
Copy link
Contributor

I'm temporary closing this PR (it is crashing the ORP spreadsheet tool). @gsorrentino18 please let me know when you have news from EGM

@agrohsje
Copy link

agrohsje commented May 4, 2021

Hi @silviodonato which news from EGM you mean?

@silviodonato
Copy link
Contributor

@gsorrentino18 will discuss with GEN about this and she will let us now (not EGM, I was wrong)

@agrohsje
Copy link

agrohsje commented May 4, 2021

Discussing what with us?

@gsorrentino18
Copy link
Contributor Author

Hi @silviodonato which news from EGM you mean?

Hi Alexander, I though we could discuss about this PR tomorrow at MccM. I updated the gDoc with my doubts

@agrohsje
Copy link

agrohsje commented May 4, 2021

Hi @gsorrentino18 . If you don't mind can you make a back-port to 11_3 and 11_2. Depending on the validation of 11_3 and the timescale of a new campaign we can then either use the 11_2 or the 11_3 backport. What do you think?

@gsorrentino18
Copy link
Contributor Author

Hi @gsorrentino18 . If you don't mind can you make a back-port to 11_3 and 11_2. Depending on the validation of 11_3 and the timescale of a new campaign we can then either use the 11_2 or the 11_3 backport. What do you think?

I can surely make a backport to 11_3. Is the 11_2 IB enabled as well? @silviodonato @smuzaffar

@agrohsje
Copy link

agrohsje commented May 4, 2021

11_2 should work. Silvio just suggested 11_3 as we might close soonish 11_2 but that shouldn't affect you.

@smuzaffar
Copy link
Contributor

yes @agrohsje , 11_2 IBs are active ( https://cmssdt.cern.ch/SDT/html/cmssdt-ib/ )

@gsorrentino18
Copy link
Contributor Author

That's fine for me then, I'll backport to 11_2. Thanks a lot for the help.
@silviodonato just let me know if you prefer me to make another PR or if you could reopen this one.

@silviodonato
Copy link
Contributor

@gsorrentino18 please make two new PRs for 11_3 and 11_2.
CMSSW_11_2_X will stay alive until June, at least.

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