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 new features to BeamSpotOnlineObjects #35338

Merged

Conversation

francescobrivio
Copy link
Contributor

PR description:

In #35193 it was reported that the BeamSpotOnline objects do not contain certain features that were published in DIP during Run2. This PR adds these features by extending the vector members of CondFormats/BeamSpotObjects/interface/BeamSpotOnlineObjects.h, specifically:

  • Events used in the BeamSpot fit [int]
  • Mean of the PVs distribution and error on the mean [float]
  • RMS of the PVs distribution and error on the RMS [float]
  • Max number of PVs [int]
  • Start (and end) time of the first (and last) LS used in the fit [string]
  • Start (and end) time of the first (and last) LS used in the fit [epoch in seconds]

DQM clients BeamMonitor and FakeBeamMonitor have been updated to add to the payload the new parameters for DIP. The DB logs have also been updated with the parameters.
Same as for the BeamSpot values in the FakeBeamMonitor client, all the parameters for DIP produced by the fake client are filled with random numbers coming from a TRandom3.

PR validation:

Tested running:

cmsRun DQM/Integration/python/clients/beamhltfake_dqm_sourceclient-live_cfg.py unitTest=True runNumber=3252833

on /store/express/Run2018E/ExpressPhysics/FEVT/Express-v1/000/325/283/00000/DF31860D-3EA3-2B42-A7CB-ACFE8C2810DF.root

This produces a log which contains:

...
-----------------------------------------------------
              BeamSpotOnline Data
 Beam type    = 2
       X0     = 0.118887 +/- 0 [cm]
       Y0     = -0.0745552 +/- 0 [cm]
       Z0     = 0.0711385 +/- 0 [cm]
 Sigma Z0     = 3.65177 +/- 0 [cm]
 dxdz         = 0 +/- 0 [radians]
 dydz         = 0 +/- 0 [radians]
 Beam Width X = 0.000404199 +/- 0 [cm]
 Beam Width Y = 0.000861879 +/- 0 [cm]
 Emittance X  = 0 [cm]
 Emittance Y  = 0 [cm]
 Beta star    = 0 [cm]
 Last Lumi    = 2
 Last Run     = 325283
 Last Fill    = 0
-----------------------------------------------------
[2021-09-19 21:57:18.589888] INFO: FakeBeamMonitor - Additional parameters for DIP:
[2021-09-19 21:57:18.589900] INFO: Events used in the fit: 821
[2021-09-19 21:57:18.589911] INFO: Mean PV               : 106.167
[2021-09-19 21:57:18.589926] INFO: Mean PV Error         : 9.39724
[2021-09-19 21:57:18.589938] INFO: Rms PV                : 7.43257
[2021-09-19 21:57:18.589950] INFO: Rms PV Error          : 7.42233
[2021-09-19 21:57:18.589962] INFO: Max PVs               : 108
[2021-09-19 21:57:18.589972] INFO: StartTime             : 1970.01.01 00:00:00 GMT
[2021-09-19 21:57:18.589982] INFO: StartTimeStamp        : 0
[2021-09-19 21:57:18.589993] INFO: EndTime               : 1970.01.01 00:00:01 GMT
[2021-09-19 21:57:18.590004] INFO: EndTimeStamp          : 1
...

Which shows that the new values are correectly stored and read from the BeamSpotOnline object

Backport:

A backport to 12_0_X will be provided.


FYI @sikler @mmusich @gennai @dzuolo

@francescobrivio
Copy link
Contributor Author

@ggovi please provide suggestions on whether some specific checks on the new features are needed in CondFormats/BeamSpotObjects/interface/BeamSpotOnlineObjects.h in order to preserve backward compatibility.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35338/25391

  • This PR adds an extra 40KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @francescobrivio for master.

It involves the following packages:

  • CondFormats/BeamSpotObjects (db, alca)
  • DQM/BeamMonitor (dqm)

@malbouis, @yuanchao, @pmandrik, @emanueleusai, @ahmad3213, @rvenditti, @cmsbuild, @jfernan2, @ggovi, @francescobrivio, @pbo0, @tvami can you please review it and eventually sign? Thanks.
@mmusich, @tocheng, @seemasharmafnal this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@tvami
Copy link
Contributor

tvami commented Sep 19, 2021

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bbe8e0/18747/summary.html
COMMIT: 7c7ac74
CMSSW: CMSSW_12_1_X_2021-09-19-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35338/18747/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: 3 differences found in the comparisons
  • DQMHistoTests: Total files compared: 40
  • DQMHistoTests: Total histograms compared: 3211080
  • DQMHistoTests: Total failures: 5
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3211052
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 39 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 169 log files, 37 edm output root files, 40 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

Thanks @francescobrivio
In order to properly test in Online DQM at P5, we definitely need the backport to 12_0_X

@francescobrivio
Copy link
Contributor Author

francescobrivio commented Sep 20, 2021

Thanks @francescobrivio
In order to properly test in Online DQM at P5, we definitely need the backport to 12_0_X

Yes definitely, I have a branch ready for the backport already, but first I would like to hear from @ggovi if I need to add any protection in the new getter methods!

@tvami
Copy link
Contributor

tvami commented Sep 20, 2021

@francescobrivio is the onlineDbService_->logger().logInfo() anyhow better then just to use the MessageLogger?

@francescobrivio
Copy link
Contributor Author

@francescobrivio is the onlineDbService_->logger().logInfo() anyhow better then just to use the MessageLogger?

onlineDbService_ is used to log the message on CondDB, e.g. https://cms-conddb.cern.ch/cmsDbBrowser/logs/show_O2O_log/Prod/BeamSpotOnlineLegacyPlayback/2021-09-20%2016:00:39.920912 (this is a log for the test tag used in the DQM playback system)

@tvami
Copy link
Contributor

tvami commented Sep 21, 2021

urgent

@francescobrivio
Copy link
Contributor Author

urgent

* we'd like the backport of this in the next 12_0_X

* FYI @qliphy @perrotta

@qliphy @perrotta
Also: we (BeamSpot team + RunCoordination + TRK) are going to discuss this PR in two days to understand how to threat BeamSpot pubblication on DIP properly (together with #35193).

@francescobrivio
Copy link
Contributor Author

francescobrivio commented Sep 21, 2021

hold

  • Will discuss on Thursday between BeamSpot + RunCoordination + TRK what is the plan for BeamSpot pubblication in DIP

@francescobrivio francescobrivio changed the title [RFC] Add new features to BeamSpotOnlineObjects Add new features to BeamSpotOnlineObjects Sep 24, 2021
@cmsbuild cmsbuild removed the hold label Sep 24, 2021
@ggovi
Copy link
Contributor

ggovi commented Sep 24, 2021

+1

@francescobrivio
Copy link
Contributor Author

@tvami @malbouis

  • extends BeamSpotOnlineObjects for DIP pubblication and for future improvemente of the onlineBeamSpotESProducer logic
  • Method in CondFormats contain a protection that throws an exception when trying to access unexisting objects in old paylaods
  • all tests passed

@tvami
Copy link
Contributor

tvami commented Sep 24, 2021

@mmusich
Copy link
Contributor

mmusich commented Sep 24, 2021

as a follow-up would be nice to add the new parameters to be displayed in the Payload Inspector:

std::function<int(parameters)> mycutFunctor = [this](parameters my_param) {
int ret(-999.);
switch (my_param) {
case lastLumi:
return this->m_payload->GetLastAnalyzedLumi();
case lastRun:
return this->m_payload->GetLastAnalyzedRun();
case lastFill:
return this->m_payload->GetLastAnalyzedFill();
case nTracks:
return this->m_payload->GetNumTracks();
case nPVs:
return this->m_payload->GetNumPVs();
case END_OF_TYPES:
return ret;
default:
return ret;
}
};

@francescobrivio
Copy link
Contributor Author

as a follow-up would be nice to add the new parameters to be displayed in the Payload Inspector:

std::function<int(parameters)> mycutFunctor = [this](parameters my_param) {
int ret(-999.);
switch (my_param) {
case lastLumi:
return this->m_payload->GetLastAnalyzedLumi();
case lastRun:
return this->m_payload->GetLastAnalyzedRun();
case lastFill:
return this->m_payload->GetLastAnalyzedFill();
case nTracks:
return this->m_payload->GetNumTracks();
case nPVs:
return this->m_payload->GetNumPVs();
case END_OF_TYPES:
return ret;
default:
return ret;
}
};

will add to my to do list! 😉

@jfernan2
Copy link
Contributor

+1
Backport tested in Online DQM at P5

@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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@francescobrivio
Copy link
Contributor Author

+1
Backport tested in Online DQM at P5

Just for reference, a log for this test can be seen in: https://cms-conddb.cern.ch/cmsDbBrowser/logs/show_O2O_log/Prod/BeamSpotOnlineHLTPlayback/2021-09-24%2016:02:14.900345

@qliphy
Copy link
Contributor

qliphy commented Sep 25, 2021

+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

7 participants