PPS lhcInfo split: per fill and per LS PopCon #40817

JanChyczynski · 2023-02-20T01:11:42Z

PR description:

Populator of Conditions for the new records, LHCInfoPerLS and LHCInfoPerFill introduced in PR39495, to be used instead of LHCInfo. Manages the data sources (OMS, CTPPS OnlineDB, DIP), creates objects of the new classes and writes them to the ofline database (CondDB). Heavily based on original LHCInfo PopCon.

The main mechanism is to take fills data from OMS (filtered by time and having stable beam), iterate through them and for every fill (meeting certian coditions depending on the mode and record) get data from all the sources, create payloads and write them to the offline database.

There are separate programs for ...perLS and ...perFill classess. Both of them have 2 modes: duringFill and endFill.

The duringFill mode, when ran, writes data (just 1 payload of most recent data) only for ongoing fills and only during stable beam. PerFill PopCon writes the payload only once per fill, and perLS PopCon writes a payload every time it's run during a stable beam.

The endFill processes a fill only once, after it's ended. PerFill PopCon writes payloads corresponding to all the lumisections of the fill (filtered for duplicates), and perLS PopCon writes 2 payloads: corresponding to the start, and the end of the stable beam of the fill.

The worklows are ment to be run by python scripts located in CondTools/RunInfo/python:

LHCInfoPerFillPopConAnalyzerDuringFill.py
LHCInfoPerFillPopConAnalyzerEndFill.py
LHCInfoPerLSPopConAnalyzerDuringFill.py
LHCInfoPerLSPopConAnalyzerEndFill.py

more info: presentation on LHCInfo update from AlCaDBmeeting

PR validation:

The python scripts running the workflow were tested on several defferent periods of time (set by the startTime and endTime parameters) by analysing the logs and data in the written payloads.

cd CondTools/RunInfo/python
cmsRun LHCInfoPerFillPopConAnalyzerDuringFill.py = tag=lhcInfoPerFill_during
cmsRun LHCInfoPerFillPopConAnalyzerEndFill.py = tag=lhcInfoPerFill_end
cmsRun LHCInfoPerLSPopConAnalyzerDuringFill.py = tag=lhcInfoPerLS_during
cmsRun LHCInfoPerLSPopConAnalyzerEndFill.py = tag=lhcInfoPerLS_end

To run the PopCon the PPS database keys are required (located in .cms_cond folder in path you can specify in authenticationPath argument in the python script).

The new PopCon was also used in further development (update of proton reconstruction, to be submitted in the next PR).

cmsbuild · 2023-02-20T01:21:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40817/34268

This PR adds an extra 20KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CondTools/RunInfo/interface/LHCInfoPerFillPopConSourceHandler.h modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/interface/LHCInfoPerLSPopConSourceHandler.h modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/interface/OMSAccess.h modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/plugins/BuildFile.xml modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/plugins/LHCInfoPerFillPopConAnalyzer.cc modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/plugins/LHCInfoPerLSPopConAnalyzer.cc modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/python/LHCInfoPerFillPopConAnalyzerEndFill.py modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/python/LHCInfoPerLSPopConAnalyzerEndFill.py modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/src/LHCInfoPerFillPopConSourceHandler.cc modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/src/LHCInfoPerLSPopConSourceHandler.cc modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685
- File CondTools/RunInfo/src/OMSAccess.cc modified in PR(s): PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685

cmsbuild · 2023-02-20T01:21:53Z

A new Pull Request was created by @JanChyczynski (jan_chyczynski) for master.

It involves the following packages:

CondTools/RunInfo (db)

@malbouis, @cmsbuild, @saumyaphor4252, @ggovi, @francescobrivio, @tvami can you please review it and eventually sign? Thanks.
@mmusich 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

tvami

Let me send you a partial review. I also didn't comment on each places, but there are a few recurring problems, please fix them at each occurance

CondTools/RunInfo/interface/LHCInfoPerFillPopConSourceHandler.h

CondTools/RunInfo/src/LHCInfoPerFillPopConSourceHandler.cc

CondTools/RunInfo/interface/LHCInfoPerLSPopConSourceHandler.h

CondTools/RunInfo/interface/LHCInfoPerFillPopConSourceHandler.h

tvami · 2023-02-23T13:12:56Z

CondTools/RunInfo/interface/LHCInfoPerLSPopConSourceHandler.h

+#include "CondCore/PopCon/interface/PopConSourceHandler.h"
+#include "CondFormats/RunInfo/interface/LHCInfoPerLS.h"
+#include "FWCore/ParameterSet/interface/ParameterSetfwd.h"
+#include "CondCore/CondDB/interface/Types.h"
+#include "CondTools/RunInfo/interface/OMSAccess.h"


please alpha order this

tvami · 2023-02-23T13:13:20Z

CondTools/RunInfo/src/LHCInfoPerLSPopConSourceHandler.cc

@@ -0,0 +1,535 @@
+#include "FWCore/MessageLogger/interface/MessageLogger.h"


alpha order

tvami · 2023-02-23T13:13:34Z

CondTools/RunInfo/src/LHCInfoPerLSPopConSourceHandler.cc

+}
+//L1: try with different m_dipSchema
+//L2: try with different m_name
+LHCInfoPerLSPopConSourceHandler::~LHCInfoPerLSPopConSourceHandler() {}


default as earlier

CondTools/RunInfo/src/LHCInfoPerFillPopConSourceHandler.cc

tvami · 2023-02-23T13:24:34Z

To run the PopCon the PPS database keys are required

I'm a bit confused.. In the end we dont want to upload to a private PPD DB, right? We want to upload to the main CondDB, so why are PPS specific keys needed?

vavati · 2023-02-27T16:04:44Z

@tvami : Several data are coming from PPS database -> PopCon -> LHCInfo

tvami · 2023-02-27T17:12:52Z

ciao @vavati thanks! I'm just wondering how we could have some tests in CMSSW. The bot has some credentials but I dont know if it has the PPS keys, @smuzaffar do you recall anything connected to PPS in the bot credentials?

smuzaffar · 2023-02-28T10:09:38Z

bot has ~/.cms_cond/db.key but I do not know if it contains PPS keys or not

francescobrivio · 2023-03-01T11:29:40Z

bot has ~/.cms_cond/db.key but I do not know if it contains PPS keys or not

ok I think I have fixed this (i.e. given the permission to read the needed DBs to the IB key), so the next round of tests (after the comments are addressed) should work fine.

tvami · 2023-03-01T14:01:50Z

bot has ~/.cms_cond/db.key but I do not know if it contains PPS keys or not

ok I think I have fixed this (i.e. given the permission to read the needed DBs to the IB key), so the next round of tests (after the comments are addressed) should work fine.

Great! So @JanChyczynski let's add unit tests that runs

LHCInfoPerFillPopConAnalyzerDuringFill.py
LHCInfoPerFillPopConAnalyzerEndFill.py
LHCInfoPerLSPopConAnalyzerDuringFill.py
LHCInfoPerLSPopConAnalyzerEndFill.py

in CMSSW

tvami · 2023-03-13T13:19:27Z

@JanChyczynski do you have any updates on this PR?

JanChyczynski · 2023-03-13T17:51:35Z

@JanChyczynski do you have any updates on this PR?

I made all the suggested changes but I'm still working on the unit tests.

JanChyczynski · 2023-04-04T22:23:29Z

I've just pushed a few commits with the suggested changes and the unit tests (I'll squash them after the review).
It will still need some work because of the problem with fillDescritpion I described in the answer to your comment.

cmsbuild · 2023-04-04T22:24:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40817/35042

This PR adds an extra 28KB to repository

cmsbuild · 2023-04-04T22:25:06Z

Pull request #40817 was updated. @cmsbuild, @tvami, @saumyaphor4252, @francescobrivio can you please check and sign again.

JanChyczynski · 2023-04-25T14:22:44Z

when the review will be considered done

what else is missing? I thought you addressed all comments

Nothing is missing, I addressed all the comments. I just didn't want to squash it as you might have wanted to look at the last commits in separate to see how I addressed the last issues. If you have no further comments I guess it's done and I'll go ahead and squash them

tvami · 2023-04-25T14:26:22Z

@cmsbuild , please test

cmsbuild · 2023-04-25T14:30:01Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40817/35268

This PR adds an extra 28KB to repository

cmsbuild · 2023-04-25T14:30:31Z

Pull request #40817 was updated. @tvami, @saumyaphor4252, @francescobrivio can you please check and sign again.

cmsbuild · 2023-04-25T17:04:48Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1b1fdd/32132/summary.html
COMMIT: 6b35a12
CMSSW: CMSSW_13_1_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/40817/32132/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 7 lines to the logs
Reco comparison results: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3460685
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3460657
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

tvami · 2023-04-26T12:37:49Z

+db

tests pass, we agreed to have a few follow-up PR

cmsbuild · 2023-04-26T12:38:17Z

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

rappoccio · 2023-04-27T16:35:21Z

+1

The test was introduced in cms-sw#40817 and fails for non-amd64 architectures: it connects to Oracle databases, and proprietary libraries for that are only available for amd64.

cmsbuild added this to the CMSSW_13_1_X milestone Feb 20, 2023

cmsbuild added db-pending pending-signatures tests-pending orp-pending code-checks-pending labels Feb 20, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 20, 2023

JanChyczynski mentioned this pull request Feb 20, 2023

PPS lhcInfo split: per fill and per LS PopCon (discarted) #39685

Closed

JanChyczynski changed the title ~~lhcInfo split: per fill and per LS PopCon~~ PPS lhcInfo split: per fill and per LS PopCon Feb 20, 2023

JanChyczynski marked this pull request as ready for review February 21, 2023 23:30

tvami reviewed Feb 23, 2023

View reviewed changes

grzanka mentioned this pull request Feb 28, 2023

Retrieval of PPS diamond detector mapping from XML and CondDB #40765

Closed

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 4, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 4, 2023

lhcInfo split: per fill and per LS PopCon

6b35a12

JanChyczynski force-pushed the jc_lhcInfo_split_PopCon_PR branch from 9699633 to 6b35a12 Compare April 25, 2023 14:23

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 25, 2023

cmsbuild added tests-started and removed tests-pending labels Apr 25, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 25, 2023

cmsbuild added tests-approved and removed tests-started labels Apr 25, 2023

cmsbuild added fully-signed db-approved and removed db-pending pending-signatures labels Apr 26, 2023

cmsbuild added orp-approved and removed orp-pending labels Apr 27, 2023

cmsbuild merged commit 7fefea9 into cms-sw:master Apr 27, 2023
11 checks passed

cmsbuild mentioned this pull request Apr 28, 2023

Displaced regional tracking iteration #40643

Merged

iarspider mentioned this pull request Apr 28, 2023

Disable CondToolsLHCInfoNewPopConTest for non-AMD64 #41455

Merged

JanChyczynski mentioned this pull request Jul 6, 2023

[13_0_X] lhcInfo split: per fill and per LS PopCon #42197

Merged

Glitchmin mentioned this pull request Aug 8, 2023

CTPPS reco update to use LHCInfoPer* records #42515

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PPS lhcInfo split: per fill and per LS PopCon #40817

PPS lhcInfo split: per fill and per LS PopCon #40817

JanChyczynski commented Feb 20, 2023 •

edited

cmsbuild commented Feb 20, 2023

cmsbuild commented Feb 20, 2023

tvami left a comment

tvami Feb 23, 2023

tvami Feb 23, 2023

tvami Feb 23, 2023

tvami commented Feb 23, 2023

vavati commented Feb 27, 2023

tvami commented Feb 27, 2023

smuzaffar commented Feb 28, 2023 •

edited

francescobrivio commented Mar 1, 2023

tvami commented Mar 1, 2023

tvami commented Mar 13, 2023

JanChyczynski commented Mar 13, 2023

JanChyczynski commented Apr 4, 2023

cmsbuild commented Apr 4, 2023

cmsbuild commented Apr 4, 2023

JanChyczynski commented Apr 25, 2023

tvami commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

tvami commented Apr 26, 2023

cmsbuild commented Apr 26, 2023

rappoccio commented Apr 27, 2023

		@@ -0,0 +1,535 @@
		#include "FWCore/MessageLogger/interface/MessageLogger.h"

PPS lhcInfo split: per fill and per LS PopCon #40817

PPS lhcInfo split: per fill and per LS PopCon #40817

Conversation

JanChyczynski commented Feb 20, 2023 • edited

PR description:

PR validation:

cmsbuild commented Feb 20, 2023

cmsbuild commented Feb 20, 2023

tvami left a comment

Choose a reason for hiding this comment

tvami Feb 23, 2023

Choose a reason for hiding this comment

tvami Feb 23, 2023

Choose a reason for hiding this comment

tvami Feb 23, 2023

Choose a reason for hiding this comment

tvami commented Feb 23, 2023

vavati commented Feb 27, 2023

tvami commented Feb 27, 2023

smuzaffar commented Feb 28, 2023 • edited

francescobrivio commented Mar 1, 2023

tvami commented Mar 1, 2023

tvami commented Mar 13, 2023

JanChyczynski commented Mar 13, 2023

JanChyczynski commented Apr 4, 2023

cmsbuild commented Apr 4, 2023

cmsbuild commented Apr 4, 2023

JanChyczynski commented Apr 25, 2023

tvami commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

Comparison Summary

tvami commented Apr 26, 2023

cmsbuild commented Apr 26, 2023

rappoccio commented Apr 27, 2023

JanChyczynski commented Feb 20, 2023 •

edited

smuzaffar commented Feb 28, 2023 •

edited