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

[13.0.X] Add SiPixelRawDataError monitoring for gpuValidation setup #41939

Merged

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Jun 13, 2023

backport of #41920

PR description:

Somewhat inspired by the discussion at #41715 (comment), the main goal of this PR is to add a DQMEDAnalyzer (SiPixelPhase1RawDataErrorComparator) designed to monitor the difference in terms of SiPixel Raw data errors in the CPU and GPU branches of the workflows running the gpuValidation modifier.
As there is no SoA-like data structure used in the CPU workflow (it reads directly the legacy digis) at variance with other heterogeneous data products used in the pixel workflow (recHits, tracks, vertices) I designed the module in order to read the legacy data format SiPixelRawDataError (converted from SoA in the GPU workflow and run directly in the CPU one).
I profit of this PR to fix a couple of bugs left from the portable data migration PR:

  • fix a mistake in one of the input tags in SiPixelCompareRecHitsSoA
  • use in SiPixelCompareRecHitsSoA the convention Host -> CPU and Device -> GPU as used in other plugins of this package

PR validation:

Successfully run the worfklow runTheMatrix.py -l 10824.507 --command='-n 100'.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Verbatim backport of #41920 to 13.0.X to be proposed in the 2023 HLT menus.

 - add comparison histograms
 - add warning messages, add total FED errors hisogram, add comments on input parametets
 - add 2D map of GPU/CPU error unbalance vs FEDid / FEDerror type
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 13, 2023

A new Pull Request was created by @mmusich (Marco Musich) for CMSSW_13_0_X.

It involves the following packages:

  • DQM/SiPixelHeterogeneous (dqm)

@nothingface0, @emanueleusai, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@idebruyn, @threus, @fioriNTU, @jandrea 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

@mmusich
Copy link
Contributor Author

mmusich commented Jun 13, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cf6347/33117/summary.html
COMMIT: 815ac3c
CMSSW: CMSSW_13_0_X_2023-06-12-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41939/33117/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 6 lines to the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3315916
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3315894
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@missirol
Copy link
Contributor

Verbatim backport of #41920 to 13.0.X to be proposed in the 2023 HLT menus.

I could use a clarification on the plan, esp. since we plan to cut the next HLT menu soon (hopefully, this week).

Is the idea to add new DQM plugins inside the HLT menu in the Path DQM_PixelReconstruction_v ? This is not difficult, but it requires time if any of the plugins is new: a new CMSSW release, the "parsing" of the new release in ConfDB (so the db knows the new plugin), then the update of the HLT menu. This is unlikely to converge in time for the upcoming HLT menu, and it might have to go to the next menu.

Could the relevant collections (e.g. hltSiPixelDigisFromSoA ?) be added to the EventContent of the DQMGPUvsCPU stream, and then let the new DQM plugins run inside the online-DQM client ? For HLT, this would only be a simple change to one EventContent, and it could be done in time for the next menu (i.e. this week).

FYI: @cms-sw/hlt-l2 @silviodonato @fwyzard

@fwyzard
Copy link
Contributor

fwyzard commented Jun 13, 2023

I would really, really prefer to keep "Host" and "Device" rather than using "CPU" and "GPU" - because the long term goal is that the "Device" could be a CPU with different scheduling, or an FPGA, etc.

In fact, for this kind of comparisons, I think it would be better to use the terms like "Reference" and "Candidate" ?

@mmusich
Copy link
Contributor Author

mmusich commented Jun 13, 2023

I would really, really prefer to keep "Host" and "Device" rather than using "CPU" and "GPU"

sure, but then let's do it consistently and not on selected plugins.

@mmusich
Copy link
Contributor Author

mmusich commented Jun 13, 2023

I could use a clarification on the plan, esp. since we plan to cut the next HLT menu soon (hopefully, this week).

I don't have yet any concrete plans, just putting this forward, in order to be able to do so, when the opportunity comes.
For the moment I just wanted to have it in order to look better at the effects described in #41715 (comment) offline.

Having said that about this:

Could the relevant collections (e.g. hltSiPixelDigisFromSoA ?) be added to the EventContent of the DQMGPUvsCPU stream, and then let the new DQM plugins run inside the online-DQM client ? For HLT, this would only be a simple change to one EventContent, and it could be done in time for the next menu (i.e. this week).

the collections are actually:

  • siPixelDigis@cuda for device (need to check how is called inside the HLT)
  • siPixelDigis@cpu for host (need to check how is called inside the HLT)

and I think what you propose makes sense, since these are legacy data-formats and can be already persisted.
On the other hand I need to setup an online client that reads them off.

@emanueleusai
Copy link
Member

+1

  • ok from DQM independently of the discussion on naming of the collections (can also be done at a later time in a separate PR from our side)

@emanueleusai
Copy link
Member

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_13_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_13_2_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 7a98008 into cms-sw:CMSSW_13_0_X Jun 14, 2023
@mmusich mmusich deleted the sipixelphase1rawdatacompartor_13_0_X branch June 14, 2023 21:27
@mmusich
Copy link
Contributor Author

mmusich commented Jun 14, 2023

a certification that an agreement on the naming conventions has been reached.

thanks Andrea, indeed, I will modify all the plugins in this package according to that suggestion in another PR in master at earliest.

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