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

BMTF-KMTF DQM Algo Selector for the DQM/L1TMonitor Subsystem #24555

Conversation

panoskatsoulis
Copy link
Contributor

This is the introduction of a new DQM plugin that selects the correct output muons of the BMTF depending on the algorithm that is triggering (Kalman BMTF/ Legacy BMTF).
Also, the PR includes the last BMTF Unpacker which is required by the DQM workflow and by the L1TBMTFAlgoSelector to operate correctly

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 15, 2018

A new Pull Request was created by @panoskatsoulis for CMSSW_10_2_X.

It involves the following packages:

DQM/L1TMonitor
DQM/L1TMonitorClient
EventFilter/L1TRawToDigi
L1Trigger/L1TMuon

@cmsbuild, @andrius-k, @kmaeshima, @schneiml, @nsmith-, @rekovic, @jfernan2, @thomreis can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @thomreis this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@thomreis
Copy link
Contributor

Hi @panoskatsoulis
does a PR to the master branch exist already?

@thomreis
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 15, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30432/console Started: 2018/09/15 19:41

@panoskatsoulis
Copy link
Contributor Author

Hi @thomreis , no I have not yet made a PR to the master.
Vladimir told me that we need a PR to 10_2_X for the tag DQM collector and I forgot.
So I'm creating one more for the master and making this a backport, correct?

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2987179
  • DQMHistoTests: Total failures: 24
  • DQMHistoTests: Total nulls: 35
  • DQMHistoTests: Total successes: 2986930
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 92.35 KiB( 30 files compared)
  • DQMHistoSizes: changed ( 136.731,... ): 4.359 KiB L1TEMU/L1TdeStage2BMTF
  • DQMHistoSizes: changed ( 136.731,... ): 2.233 KiB L1T/L1TStage2BMTF
  • DQMHistoSizes: changed ( 10042.0,... ): 4.363 KiB L1TEMU/L1TdeStage2BMTF
  • Checked 129 log files, 14 edm output root files, 31 DQM output files

@panoskatsoulis
Copy link
Contributor Author

@andrius-k
I've done the change to the other PR, so if this is what you need I will make the change here as well.

@thomreis
I think the problem with the code-checks has been corrected. It seems was related only with the other branch, I just did the same proposed changes here as well because they were good coding practice.

Please wait because we need to merge in this branch the last Kalman Emulator too.
We want to use it during the weekend low lumi tests.
I'm waiting for @bachtis to give me the Emulator and I will integrate it here.
Thanks

@panoskatsoulis panoskatsoulis changed the title BMTF-KMTF DQM Algo Selector BMTF-KMTF DQM Algo Selector for the DQM/L1TMonitor Subsystem Sep 17, 2018
@thomreis
Copy link
Contributor

@panoskatsoulis the code checks are only run for the master branch. But you should always backport all commits for the master branch to the backport PRs.

@thomreis
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 18, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30479/console Started: 2018/09/18 16:03

@panoskatsoulis
Copy link
Contributor Author

backport of #24557

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@andrius-k
Copy link

@panoskatsoulis Are you sure this is a proper backport? The changes of this PR and the original one seem to be different, so I'm just double checking.

@panoskatsoulis
Copy link
Contributor Author

@andrius-k
The number of files are different because in one of the PRs a rename of a file has been treated as deletion and creation to the other.

Please don't merge these yet, we need to update them including the latest (to the date) available Emulator of the Kalman Algorithm. We need to take it to P5 for the tests too.

@andrius-k
Copy link

@panoskatsoulis Which file in particular are you talking about?

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2984698
  • DQMHistoTests: Total failures: 22
  • DQMHistoTests: Total nulls: 32
  • DQMHistoTests: Total successes: 2984454
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 92.302 KiB( 30 files compared)
  • DQMHistoSizes: changed ( 1306.0,... ): 4.359 KiB L1TEMU/L1TdeStage2BMTF
  • DQMHistoSizes: changed ( 1306.0,... ): 2.233 KiB L1T/L1TStage2BMTF
  • Checked 129 log files, 14 edm output root files, 31 DQM output files

@panoskatsoulis
Copy link
Contributor Author

@andrius-k
I'm looking at it, seems that I have forgotten to remove a file
I'm talking for the file "DQM/L1TMonitor/python/L1TdeStage2kBMTF_cff.py"
It has been changed into this: "DQM/L1TMonitor/python/L1TdeStage2BMTFSecond_cff.py "

@andrius-k
Copy link

@panoskatsoulis but if the same changes were made (file renames, etc.), diffs should mach shouldn't they? Anyway please report your findings here.

@panoskatsoulis
Copy link
Contributor Author

Hi @andrius-k ,
from my understanding 10_2_X is not that updated as the master because of an open cms-l1t-offline PR. @rekovic can correct me on that.
So this is why the diffs are different. Master branch include newer code that this 10_2_X.
But I am trying to merge the same code, and so the diffs are different.

Panos

@andrius-k
Copy link

Hi @panoskatsoulis
Does it mean that this PR is a backport of two PRs? In that case could you please mark it like that?
Also the history of commits between the original PR and the backport seem to be really different. Is it because the backport was made manually?

@panoskatsoulis
Copy link
Contributor Author

panoskatsoulis commented Sep 26, 2018

Hi, all
there is a newer version if this code that fixes the SEGFAULT which mentioned in the PR #24557
The new PR is #24660

This PR will be closed if not needed any more

@andrius-k
Copy link

Hi @panoskatsoulis, yes, please close this.

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.

4 participants