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

Add displaced muon info from EMTF to RegionalMuonCand (un)packing and to uGMT DQM #33146

Merged

Conversation

dinyar
Copy link
Contributor

@dinyar dinyar commented Mar 11, 2021

PR description:

This PR adds awareness of displaced muon information from EMTF to the RegionalMuonCand (un)packers. It also adds plots for these data to the uGMT DQM.

PR validation:

I ran runTheMatrix.py -l limited -i all --ibeos and scram b runtests without errors. I also checked that the plots in the DQM appeared as expected.

@eyigitba please check if this makes sense to you too. I think you may need to update the EMTF (un)packers accordingly if this hasn't been done already.
attn @rekovic, please let me know if you'd like me to backport this.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33146/21517

ERROR: Build errors found during clang-tidy run.

L1Trigger/L1TNtuples/src/L1AnalysisL1UpgradeTfMuon.cc:64:24: error: too many arguments to function call, expected 2, have 3 [clang-diagnostic-error]
            *it, true, false));  // TODO: We're assuming that we're dealing with Kalman muons here.
                       ^
L1Trigger/L1TMuon/interface/RegionalMuonRawDigiTranslator.h:17:5: note: 'generateRawTrkAddress' declared here
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:128: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33146/21518

  • This PR adds an extra 52KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @dinyar (Dinyar Rabady) for master.

It involves the following packages:

DQM/L1TMonitor
EventFilter/L1TRawToDigi
L1Trigger/L1TMuon

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @rekovic, @jfernan2, @cecilecaillol, @rvenditti can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @thomreis 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

@jfernan2
Copy link
Contributor

@dinyar can you please add yourself along with your github username to the corresponding e-group in

https://twiki.cern.ch/twiki/bin/viewauth/CMS/DQMContacts#L1T

?

Thanks

@dinyar
Copy link
Contributor Author

dinyar commented Mar 11, 2021

Hi @jfernan2,

I added myself to the online developers list a few days ago (however didn't add my git username). I've now added myself to the offline developers list too.

Cheers,
Dinyar

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9ddd16/13452/summary.html
COMMIT: d194d54
CMSSW: CMSSW_11_3_X_2021-03-11-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33146/13452/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2635087
  • DQMHistoTests: Total failures: 25
  • DQMHistoTests: Total nulls: 12
  • DQMHistoTests: Total successes: 2635028
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 37.218 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 8.910 KiB L1T/L1TStage2uGMT
  • DQMHistoSizes: changed ( 11634.0,... ): 3.496 KiB L1TEMU/L1TdeStage2uGMT
  • Checked 155 log files, 37 edm output root files, 37 DQM output files

@jfernan2
Copy link
Contributor

+1

@eyigitba
Copy link
Contributor

Hi @dinyar sorry for the delayed response. I think these make sense to me, thanks! Like you said we need to update our unpacker/packer in a similar way as well. I'll let you know once we're doing that.

@rekovic
Copy link
Contributor

rekovic commented Mar 16, 2021

@dinyar We are still expecting commits to this PR, correct ?

@dinyar
Copy link
Contributor Author

dinyar commented Mar 16, 2021

Hi @rekovic, no sorry I didn't state this but 24d6ff9 should do the trick.

Cheers,
Dinyar

@rekovic
Copy link
Contributor

rekovic commented Mar 17, 2021

+1

@rekovic
Copy link
Contributor

rekovic commented Mar 17, 2021

@dinyar Please make a backport 11_2_X PR.

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9ddd16/13572/summary.html
COMMIT: 24d6ff9
CMSSW: CMSSW_11_3_X_2021-03-16-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33146/13572/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2639881
  • DQMHistoTests: Total failures: 36
  • DQMHistoTests: Total nulls: 13
  • DQMHistoTests: Total successes: 2639810
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 37.214 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 8.910 KiB L1T/L1TStage2uGMT
  • DQMHistoSizes: changed ( 11634.0,... ): 3.496 KiB L1TEMU/L1TdeStage2uGMT
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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 now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@silviodonato
Copy link
Contributor

+1

@boudoul
Copy link
Contributor

boudoul commented Mar 31, 2021

Hello @dinyar : The runthematrix -limited is actually not using run3 data (maybe one should add this btw, but this is a separate question) , therefore my question is : how confident are you in using this code (backported in 11_2_X) for the next MWGR? There is a workflow using previous run3 data in runthematrix , would that help to run it in order to spot possible issue/bugs?
Thanks .
(@amassiro FYI)

@dinyar
Copy link
Contributor Author

dinyar commented Mar 31, 2021

Hi Gaelle,

Ah, I think I might be confused then, I checked the presence of the DQM plots in 11634.0_TTbar_14TeV+2021+TTbar_14TeV_TuneCP5_GenSimINPUT+Digi+Reco+HARVEST+ALCA/DQM_V0001_R000000001__Global__CMSSW_X_Y_Z__RECO.root which at least uses the Run3 era (which I think also causes EMTF to reconstruct displaced muons, at least I see a couple of them in the plots). I then confirmed in 1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15INPUT+DIGIUP15+RECOUP15+HARVESTUP15/DQM_V0001_R000000001__Global__CMSSW_X_Y_Z__RECO.root that the plots don't appear for the Run2 eras. Did you mean something else?

Regardless, I think I can add some context regarding the backport: It's not strictly needed for the upcoming MWGR because EMTF will not be deploying their new firmware then (though @eyigitba should confirm if this is still the case). The only issue I can imagine is if EMTF is sending information on the bits reserved for the displaced data: In this case I would expect EMTF output vs. uGMT input mismatches as well as uGMT data-emulator mismatches (if I don't deploy a uGMT version that enables the displaced data from EMTF). L1T offline software coordination requested I backport this to have it in 11_2_X in case 11_3_X were delayed for the May MWGR, so for me it's fine to delay until after the upcoming one if you prefer that. Maybe @rekovic or @cecilecaillol want to comment further.

Cheers,
Dinyar

@boudoul
Copy link
Contributor

boudoul commented Mar 31, 2021

Hello @dinyar , thanks for your prompt feedback !
I'm actually not trying to delay this integration , but I just would like understand if we explore all aspects before using it in data taking (if you postpone to a next MWGR, i'll come anyway with the same questions :-) )
For your tests : what you did is just fine : checking the DQM plots present in the run3 era and not in the run2 era is perfect. However those workflows are MonteCarlo. I'm not worried by the DQM changes, more on the unpacker side, and therefore I was wondering whether the run3 Data (WFs 138.1 and 138.2 ) could be sensitive to your change (but maybe not?) and in that case whether you tried to run them . Thanks.

@dinyar
Copy link
Contributor Author

dinyar commented Mar 31, 2021

Ah, understood. Thanks for the clarification, I wasn't aware there were workflows that used actual run3 data already.

I didn't try to run those (as is probably apparent from my answers :) ), but I now saw that @jfernan2 did a replay at P5 in #33311. It looks like this went fine, but if someone could point me at the DQM GUI for the replay I would check to make sure that the plots still look as expected.

Cheers,
Dinyar

@eyigitba
Copy link
Contributor

eyigitba commented Apr 1, 2021

Hi @dinyar , we didn't specify an era setting for displaced information in emulator. It should be working in the same way for Run 3 and Run 2. Now that you mentioned maybe it's a good idea to add that distinction.

About the MWGR and displaced information, as far as I know we won't update the firmware for this MWGR. I'll keep you updated about the future ones. I don't know if we're currently sending data on the bits reserved for displaced information. I'll try to ask Alex again.

@jfernan2
Copy link
Contributor

jfernan2 commented Apr 1, 2021

I didn't try to run those (as is probably apparent from my answers :) ), but I now saw that @jfernan2 did a replay at P5 in #33311. It looks like this went fine, but if someone could point me at the DQM GUI for the replay I would check to make sure that the plots still look as expected.

@dinyar you can check for example run 340220 in
https://cmsweb.cern.ch/dqm/online-playback/
HTH

@dinyar
Copy link
Contributor Author

dinyar commented Apr 6, 2021

Thanks a lot, @jfernan2! I checked and things look mostly fine for me. One issue I spotted is that it seems that EMTF is indeed sending some data in the fields foreseen for unconstrained pT and dXY leading to data-emulator mismatches for uGMT (I mentioned this above as a possibility). I'm currently confirming with Alex whether this is expected.

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

7 participants