fractional prescale propagation into L1-O2O in 112 x #30886

hjkwon260 · 2020-07-23T16:01:57Z

PR description:

To adapt fractional prescale into the L1 O2O payload, data type of the prescale is changed from the integer.
This update involves change in CondFormats, so everywhere fetch the prescale in L1T codes is also updated to success the compilation.

We (L1 O2O team, @gekobs @panoskatsoulis ) understand the changes would involve the needed change for uGT emulator. So I ping @cavana @zhenbinwu of this PR.

PR validation:

The code has been tested running the runTheMatrix workflows, code-checks and code-format.
Local test on O2O was done using tsc/rs key containing fractional prescale, checked the payload contains the fractional vaules.

Backport 11_1_X: #31545

cmsbuild · 2020-07-23T16:02:21Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-07-23T16:08:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30886/17269

This PR adds an extra 52KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File L1Trigger/L1TGlobal/src/L1TGlobalUtil.cc modified in PR(s): [Clang tidy] Apply checks for l1 #30802
- File L1TriggerConfig/L1TConfigProducers/src/L1TGlobalPrescalesVetosOnlineProd.cc modified in PR(s): [Clang tidy] Apply checks for l1 #30802

cmsbuild · 2020-07-23T16:09:05Z

A new Pull Request was created by @hjkwon260 (Hyejin Kwon) for master.

It involves the following packages:

CondFormats/L1TObjects
L1Trigger/L1TGlobal
L1TriggerConfig/L1TConfigProducers

@benkrikler, @christopheralanwest, @tocheng, @cmsbuild, @rekovic, @tlampen, @ggovi, @pohsun can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @mmusich, @seemasharmafnal, @tocheng this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

ggovi · 2020-07-24T14:05:50Z

CondFormats/L1TObjects/interface/L1TGlobalPrescalesVetos.h

@@ -18,7 +18,7 @@ class L1TGlobalPrescalesVetos {
  }

  unsigned int version_;
-  std::vector<std::vector<int> > prescale_table_;
+  std::vector<std::vector<double> > prescale_table_;


@hjkwon260 You can't change data attributes types/names, since the back-ward compatibility in reading existing data would be broken.
If this change is required, you should create a new class.

@ggovi thank you very much for the comment! I see, I'll create a new class and propagate it to o2o producer

boudoul · 2020-07-27T14:36:48Z

Dear @hjkwon260 , is this something that would be needed for the next MWGR ? (scheduled first week of September) - Thanks

hjkwon260 · 2020-07-27T14:59:22Z

Dear @boudoul Yes, we are planning so, thank you

silviodonato · 2020-08-05T08:47:19Z

please test

silviodonato · 2020-08-05T08:47:33Z

do you have any news @hjkwon260 ?

cmsbuild · 2020-08-05T08:47:40Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-08-04-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/8572/console Started: 2020/08/05 12:07

cmsbuild · 2020-08-05T11:39:21Z

+1
Tested at: 92820a9
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d38d6d/8572/summary.html
CMSSW: CMSSW_11_2_X_2020-08-04-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-08-05T11:39:23Z

Comparison job queued.

cmsbuild · 2020-08-05T15:10:17Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 2612401
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2612347
DQMHistoTests: Total skipped: 48
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 149 log files, 22 edm output root files, 35 DQM output files

tlampen · 2020-09-08T11:16:36Z

+1

boudoul · 2020-09-08T12:05:59Z

@hjkwon260 @gekobs @panoskatsoulis @cavana @zhenbinwu @BenjaminRS @bundocka @rekovic : What is the plan for testing this PR prior to the next MWGR ? Adding aslo @tapio, @christopheralanwest as they are presumably part of builing this plan for testing . Thanks

BenjaminRS · 2020-09-08T12:26:42Z

HI @boudoul -- we are also currently discussing with TSG and FOG to see what else is needed for this to go in. But it still needs sign off by various groups.

boudoul · 2020-09-08T12:45:46Z

Very good, thanks @BenjaminRS

silviodonato · 2020-09-08T17:07:02Z

@cms-sw/db-l2 @cms-sw/l1-l2

Sam-Harper · 2020-09-13T18:20:24Z

So as requested we did a little bit of investigating in the HLT, using PrintEventSetupDataRetrieval, we can see which es products the HLT is actually consuming.

This should be a sufficient test. The conclusions are: the HLT as run online does not consume the prescale record and therefore this change can have no effect. This has been tested on both a full GRun menu offline and the current cosmics menu on hilton

However when run for development, one of our analyser paths does consume this via L1TGlobalUtil. Additionally L1TGlobalProducer may be optionally configured to read the prescale information.

Both of these code are L1 owned and therefore the L1 group should ideally update this code before progressing as without doing will limit our ability to analyse the prescale information of runs taken with this offline. In the interests of expediency and given the current effects are minor, I am willing to instead take the promise of the L1 group to submit this PR within a month after the MGWR.

When this is done there is the requirement that inside a CMSSW job we should be able to access the prescale information of any data we run over, thus the solutions should ensure that we can still read the precale information of the old data. I would also encourage you to find a solution that ensures that this is as transparent and robust as possible. For example, I would encourage you to write an iov in the existing record which is set to a dumy value (all 0 prescales or something) so it is absolutely clear to a user that they are now picking up the wrong record. I also think the simpliest approach would be to convert all the existing records for the data into the new format, this I strongly suspect could be easily achieved with a day or two worth of effort with a simple script. We would therefore only have one record to worry about rather than having to fall back to the old one via some error prone mechanism.

I assume you are in full consultation with the OMS developers on this change.

ggovi · 2020-09-15T13:30:33Z

+1

silviodonato · 2020-09-16T12:41:43Z

Kind reminder @cms-sw/l1-l2

silviodonato · 2020-09-21T10:24:28Z

@rekovic this PR is waiting for a review/signature for a long time. We need to merge this soon in order to have the backport ready for the next MWGR. Do you have any comments? Thank you

rekovic · 2020-09-22T09:12:07Z

+1

cmsbuild · 2020-09-22T09:12:35Z

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

silviodonato · 2020-09-22T09:26:04Z

+1

silviodonato · 2020-09-22T09:26:36Z

@hjkwon260 please make a backport for 11_1_X

boudoul · 2020-09-22T09:41:30Z

Hi @rekovic , we were expecting a bit more than just a '+1' . There are comments from Sam which would be good to answer/follow up . Thanks

rekovic · 2020-09-22T11:28:24Z

Hi @boudoul
For the time being, for MWGR we are good with this PR.
We are scheduling a discussion with AlCaDB (@ggovi) to address the adjustments to the L1T code which would address the need of accessing these prescales from DB in an Offline use mentioned by @Sam-Harper.

Martin-Grunewald · 2020-09-28T12:05:44Z

Hi,

I now get a missing L1GtPrescaleFactorsAlgoTrigTrivialProducer but I do not see it removed in this PR:

%MSG
----- Begin Fatal Exception 28-Sep-2020 13:37:39 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'L1GtPrescaleFactorsAlgoTrigTrivialProducer' in category \
'CMS EDM Framework ParameterSet Description'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

Martin-Grunewald · 2020-09-28T14:00:25Z

Solved by #31593

cmsbuild added this to the CMSSW_11_2_X milestone Jul 23, 2020

cmsbuild added alca-pending code-checks-pending comparison-pending db-pending l1-pending orp-pending pending-signatures tests-pending labels Jul 23, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 23, 2020

ggovi reviewed Jul 24, 2020

View reviewed changes

This was referenced Jul 28, 2020

HDF5 interface for LHE-HDF5 event files #30312

Merged

BS online for 11 2 x #30591

Closed

Add Phase 2 Trackers T21 + T22 + T23 (Mechanical Update in Outer Tracker + Sensors studies in Inner Tracker) #30927

Closed

cmsbuild added tests-started and removed tests-pending labels Aug 5, 2020

cmsbuild added tests-approved and removed tests-started labels Aug 5, 2020

cmsbuild added alca-approved and removed alca-pending labels Sep 8, 2020

cmsbuild added db-approved and removed db-pending labels Sep 15, 2020

cmsbuild added fully-signed l1-approved and removed l1-pending pending-signatures labels Sep 22, 2020

cmsbuild added orp-approved and removed orp-pending labels Sep 22, 2020

cmsbuild merged commit 36738c9 into cms-sw:master Sep 22, 2020

cmsbuild mentioned this pull request Sep 22, 2020

[DD4hep] DDFilteredView API Enhancement and Tracker Configuration by Era #31482

Closed

hjkwon260 mentioned this pull request Sep 22, 2020

Backport to 111X of L1-O2O fractional prescale propagation #31545

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fractional prescale propagation into L1-O2O in 112 x #30886

fractional prescale propagation into L1-O2O in 112 x #30886

hjkwon260 commented Jul 23, 2020 •

edited by silviodonato

cmsbuild commented Jul 23, 2020

cmsbuild commented Jul 23, 2020

cmsbuild commented Jul 23, 2020

ggovi Jul 24, 2020

hjkwon260 Jul 27, 2020

boudoul commented Jul 27, 2020 •

edited

hjkwon260 commented Jul 27, 2020

silviodonato commented Aug 5, 2020

silviodonato commented Aug 5, 2020

cmsbuild commented Aug 5, 2020 •

edited

cmsbuild commented Aug 5, 2020

cmsbuild commented Aug 5, 2020

cmsbuild commented Aug 5, 2020

tlampen commented Sep 8, 2020

boudoul commented Sep 8, 2020

BenjaminRS commented Sep 8, 2020

boudoul commented Sep 8, 2020

silviodonato commented Sep 8, 2020

Sam-Harper commented Sep 13, 2020

ggovi commented Sep 15, 2020

silviodonato commented Sep 16, 2020

silviodonato commented Sep 21, 2020

rekovic commented Sep 22, 2020

cmsbuild commented Sep 22, 2020

silviodonato commented Sep 22, 2020

silviodonato commented Sep 22, 2020

boudoul commented Sep 22, 2020

rekovic commented Sep 22, 2020

Martin-Grunewald commented Sep 28, 2020

Martin-Grunewald commented Sep 28, 2020

fractional prescale propagation into L1-O2O in 112 x #30886

fractional prescale propagation into L1-O2O in 112 x #30886

Conversation

hjkwon260 commented Jul 23, 2020 • edited by silviodonato

PR description:

PR validation:

cmsbuild commented Jul 23, 2020

cmsbuild commented Jul 23, 2020

cmsbuild commented Jul 23, 2020

ggovi Jul 24, 2020

Choose a reason for hiding this comment

hjkwon260 Jul 27, 2020

Choose a reason for hiding this comment

boudoul commented Jul 27, 2020 • edited

hjkwon260 commented Jul 27, 2020

silviodonato commented Aug 5, 2020

silviodonato commented Aug 5, 2020

cmsbuild commented Aug 5, 2020 • edited

cmsbuild commented Aug 5, 2020

cmsbuild commented Aug 5, 2020

cmsbuild commented Aug 5, 2020

tlampen commented Sep 8, 2020

boudoul commented Sep 8, 2020

BenjaminRS commented Sep 8, 2020

boudoul commented Sep 8, 2020

silviodonato commented Sep 8, 2020

Sam-Harper commented Sep 13, 2020

ggovi commented Sep 15, 2020

silviodonato commented Sep 16, 2020

silviodonato commented Sep 21, 2020

rekovic commented Sep 22, 2020

cmsbuild commented Sep 22, 2020

silviodonato commented Sep 22, 2020

silviodonato commented Sep 22, 2020

boudoul commented Sep 22, 2020

rekovic commented Sep 22, 2020

Martin-Grunewald commented Sep 28, 2020

Martin-Grunewald commented Sep 28, 2020

hjkwon260 commented Jul 23, 2020 •

edited by silviodonato

boudoul commented Jul 27, 2020 •

edited

cmsbuild commented Aug 5, 2020 •

edited