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 layer-1 monitoring for new slot-7 cards [13_0_3 P5 DQM test] #41383

Closed
wants to merge 1 commit into from

Conversation

hftsoi
Copy link
Contributor

@hftsoi hftsoi commented Apr 21, 2023

PR description:

This PR modifies Calo-Layer1 unpacker to adapt the additions of a new CTP7 card in slot-7 in each of the three layer-1 crates (FEDs 1354, 1356, 1358), where each card sends the same payload header and trailer as all other existing calo cards, but with a fixed payload data size of 6 32-bit words, regardless of normal or FAT events being sent. New monitoring elements are added to layer-1 DQM for the 3x6x32 bits. The modification is done in such a way that it works before and after the card addition (current firmware and new firmware not yet uploaded).

Note that the monitoring elements for HCAL FB4-5 are commented out, we will put them back once HCAL fixes them (FB4-5 are reserved bits and not used for LLP, but they are sending unphysical data there which layer-1 could not read out, causing discrepancies seen when comparing them).

PR validation:

Validated by running offline DQM on past commissioning runs, it works as expected for current production firmware. It needs to be tested with new firmware not yet deployed at P5.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @hftsoi (Ho-Fung Tsoi) for CMSSW_13_0_X.

It involves the following packages:

  • DQM/L1TMonitor (dqm)
  • EventFilter/L1TRawToDigi (l1)

@aloeliger, @epalencia, @emanueleusai, @cmsbuild, @syuvivida, @pmandrik, @micsucmed, @cecilecaillol, @rvenditti can you please review it and eventually sign? Thanks.
@dinyar, @missirol, @Martin-Grunewald, @thomreis, @eyigitba this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@hftsoi
Copy link
Contributor Author

hftsoi commented Apr 21, 2023

This PR is opened for online DQM test at P5, not yet finalized, please ignore for the moment.

@aloeliger
Copy link
Contributor

@hftsoi Could you please move this PR to draft to indicate from your side that this is testing only?

@micsucmed
Copy link

Hi, the PR has been added to DQM playback systems could you please check the changes are as expected, thank you

@hftsoi hftsoi marked this pull request as draft April 24, 2023 14:36
@hftsoi
Copy link
Contributor Author

hftsoi commented Apr 24, 2023

hi @micsucmed , the playback plots look as expected. We will postpone the test till later this week as we don't want to disrupt the current runs with beams, and will let you know when we are ready to have this PR in online production. Thanks

@emanueleusai
Copy link
Member

in the meantime we should trigger the tests and, if this is a permanent change, you can prepare the corresponding PRs for master and 13_1 at your convenience. Release managers will usually not merge the backport before the master PR.

@emanueleusai
Copy link
Member

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-607a6f/32135/summary.html
COMMIT: 40de051
CMSSW: CMSSW_13_0_X_2023-04-25-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41383/32135/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 140.049140.049_RunTau2022C/step2_RunTau2022C.log
  • 140.048140.048_RunEGamma2022C/step2_RunEGamma2022C.log
  • 140.051140.051_RunMuonEG2022C/step2_RunMuonEG2022C.log
Expand to see more relval errors ...

Comparison Summary

Summary:

  • You potentially removed 15 lines from the logs
  • Reco comparison results: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3554258
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3554230
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -925.9400000000003 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -46.297 KiB L1T/L1TStage2CaloLayer1
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@emanueleusai
Copy link
Member

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-607a6f/32309/summary.html
COMMIT: 40de051
CMSSW: CMSSW_13_0_X_2023-05-02-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41383/32309/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 35 lines from the logs
  • Reco comparison results: 17 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3554724
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3554691
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -925.9400000000003 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 12434.0,... ): -46.297 KiB L1T/L1TStage2CaloLayer1
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@aloeliger
Copy link
Contributor

@hftsoi I wanted to check in on the status of this. Is this ready to go out of draft and be integrated?

@hftsoi
Copy link
Contributor Author

hftsoi commented May 4, 2023

@hftsoi I wanted to check in on the status of this. Is this ready to go out of draft and be integrated?

Not yet, so far it has never tested with the new firmware which is not ready now

@syuvivida
Copy link
Contributor

@hftsoi is the new firmware ready now? any plan to make this PR official?
Thanks!!

Eiko

@hftsoi
Copy link
Contributor Author

hftsoi commented Jun 12, 2023

@hftsoi is the new firmware ready now? any plan to make this PR official? Thanks!!

Eiko

The new firmware will be ready later this week, we shall test it with this PR together.

@syuvivida
Copy link
Contributor

@hftsoi this PR has been deployed in online DQM since when it was created. Therefore it will be tested with the new firmware.

@hftsoi
Copy link
Contributor Author

hftsoi commented Jun 19, 2023

@syuvivida an initial test was performed last week, however there was some minor issue in firmware loading during the test so the functionality was not fully examined, we will need to repeat it probably later this week. On top of this PR, I would like to add an extra check on firmware status when filling relevant plots to improve future debugging, shall I commit to this PR, or close this one and open a new one based on 13_0_7 (from current P5tag collector)? Thanks.

@syuvivida
Copy link
Contributor

Hi @hftsoi
Could you close this one and open a new one based on 13_0_7? This will be cleaner rather than using the original PR.
Thanks!!

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.

6 participants