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

Pr 111 x Displaced muon in BMTF and uGMT ( unpacker/emulator/packer) #31679

Conversation

rekovic
Copy link
Contributor

@rekovic rekovic commented Oct 6, 2020

PR description:

11_2_X: L1T Displaced muons: Pt from unconstrainedVtx

  • Unpacker/Emulator/Packer for KBMTF and uGMT (unpacker same for uGT)
  • Changes in DataFormat to accessors of the displaced-Pt and impact parameter

PR validation:

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

backport of #31608

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

dinyar and others added 30 commits October 6, 2020 10:47
The BMTF with the Kalman filter sends inverted track addresses,
the uGMT cancel-out unit has been adapted for this. In this
commit it has also been configured to use the new mode and
pick barrel muons from the KBMTF collection.
The original field with track addresses was filled with incorrect or incomplete values for most TFs.
Using static function from RegionalMuonRawDigiTranslator that I factored out in order to generate the raw track address.
May make the interface to get them more clear.
This makes us a bit more consistent with the other fields.
Co-Authored-By: panoskatsoulis <panos.katsoulis@cern.ch>
This should be configured based on eras in the future.
In the MuonUnpacker the wrong overloaded function was being called, this is fixed now.
In the process I also rewrote my earlier attempt, it should be more readable now.
Also add displacement information to uGMT intermediate muons and light
refactoring for clarity.
Packs correctly for Run-2 2016, Run-2 from 2017 (w/ extrapolated coordiantes),
and Run-3
The original field with track addresses was filled with incorrect or incomplete values for most TFs.
Using static function from RegionalMuonRawDigiTranslator that I factored out in order to generate the raw track address.
Includes configuration of packer by Era.
Modifications still needed for the BMTF setup to make use of this.
Not done nicely, should be cleaned up.
@rekovic
Copy link
Contributor Author

rekovic commented Oct 27, 2020

+1

@rekovic
Copy link
Contributor Author

rekovic commented Oct 27, 2020

backport of #31608

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19454c/10316/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2784680
  • DQMHistoTests: Total failures: 71
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2784559
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.12 KiB( 35 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): 0.047 KiB L1TEMU/L1TdeStage2BMTF
  • DQMHistoSizes: changed ( 10024.0,... ): 0.023 KiB L1T/L1TStage2BMTF
  • Checked 152 log files, 16 edm output root files, 36 DQM output files

@rekovic
Copy link
Contributor Author

rekovic commented Oct 28, 2020

@jfernan2
This updated PR is now fully in sync with the master PR. We need it for the next MWGR.
Can you please review/sign. Thanks.

//debug up to here
*/
//debug from here
std::cout << "block id : " << block.header().getID() << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

despite the code is commented and it was already there, are couts allowed?

@jfernan2
Copy link
Contributor

@rekovic since this PR is needed for next MWGR, we are testing it at P5 in DQM Playback

@jfernan2
Copy link
Contributor

The test at P5 went fine, I am OK with the PR, I am not sure if the coding rules allow the couts even if commented

@dinyar
Copy link
Contributor

dinyar commented Oct 28, 2020

Hi @jfernan2,

since this is a backport I suppose it would be best to keep it as is, but I could make a new PR to remove those lines. Would that be acceptable for you?

Cheers,
Dinyar

@jfernan2
Copy link
Contributor

Sure

@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 CMSSW_11_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_2_X is complete. 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)

@qliphy
Copy link
Contributor

qliphy commented Oct 28, 2020

+1

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

6 participants