Fix small typo in `makePileupJSON.py`: `runNumber` → `run` #45989

IzaakWN · 2024-09-12T12:46:09Z

This PR fixes small typo in https://github.com/cms-sw/cmssw/blob/master/RecoLuminosity/LumiDB/scripts/makePileupJSON.py.

Run number is introduced as just run:

cmssw/RecoLuminosity/LumiDB/scripts/makePileupJSON.py

Lines 57 to 73 in c312b85

    
           for line in csv_input: 
        
               if line[0] == '#': 
        
                   continue # skip comment lines 
        
               pieces = sepRE.split(line.strip()) 
        
               if len(pieces) < 15: 
        
                   # The most likely cause of this is that we're using a csv file without the bunch data, so might as well 
        
                   # just give up now. 
        
                   raise RuntimeError("Not enough fields in input line; maybe you forgot to include --xing in your brilcalc command?\n"+line) 
        
               try: 
        
                   run = int(pieces[0]) 
        
                   lumi_section = int(pieces[2]) 
        
                   beam_energy = float(pieces[10]) 
        
                   #tot_del_lumi = float(pieces[11]) 
        
                   #tot_rec_lumi = float(pieces[12]) 
        
                   luminometer = pieces[14]

But later runNumber is used:

cmssw/RecoLuminosity/LumiDB/scripts/makePileupJSON.py

Lines 132 to 138 in c312b85

    
           # second loop to get (weighted) RMS 
        
           for bxid, bunch_del_lumi, bunch_rec_lumi in xing_lumi_array: 
        
               mean_pileup = bunch_del_lumi * parameters.orbitLength / parameters.lumiSectionLength 
        
               if mean_pileup > 100: 
        
                   print("mean number of pileup events > 100 for run %d, lum %d : m %f l %f" % \ 
        
                         (runNumber, lumi_section, mean_pileup, bunch_del_lumi)) 
        
                   #print "mean number of pileup events for lum %d: m %f idx %d l %f" % (lumi_section, mean_pileup, bxid, bunch_rec_lumi)

This led to the following error when computing PU profile for 2024, following instructions from this "PileupJSONFileforData" TWiki:

$ makePileupJSON.py lumi_DCSONLY.csv pileup_JSON.txt I get this error:
...
Processing <...>/lumi_DCSONLY.csv with all BX
Traceback (most recent call last):
  File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/makePileupJSON.py", line 137, in <module>
    (runNumber, lumi_section, mean_pileup, bunch_del_lumi))
NameError: name 'runNumber' is not defined

cmsbuild · 2024-09-12T12:46:30Z

cms-bot internal usage

cmsbuild · 2024-09-12T12:50:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45989/41757

cmsbuild · 2024-09-12T12:50:59Z

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

It involves the following packages:

RecoLuminosity/LumiDB (db)

@cmsbuild, @consuegs, @francescobrivio, @perrotta, @saumyaphor4252 can you please review it and eventually sign? Thanks.
@JanChyczynski, @PonIlya, @missirol, @mmusich, @rsreds, @yuanchao this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

perrotta · 2024-09-12T13:18:59Z

please test
(It looks like PU above 100 was never attained in the tests before...)

IzaakWN · 2024-09-12T14:21:47Z

please test (It looks like PU above 100 was never attained in the tests before...)

After fixing the typo locally, we only see it happen four times in this Collisions24_13p6TeV_378981_385514_DCSOnly_TkPx.json file:

mean number of pileup events > 100 for run 384264, lum 862 : m 11721745844.106108 l 3072785342557351.500000
mean number of pileup events > 100 for run 384383, lum 213 : m 295925365174.010925 l 77575058928175920.000000
mean number of pileup events > 100 for run 384565, lum 642 : m 646223.555834 l 169403627820.420685
mean number of pileup events > 100 for run 385355, lum 913 : m 1230240082.388803 l 322500056157730.437500

Is this to be expected? I do not see any peak luminosities jumping out in 2024 compared to previous years in the summary plots?

IzaakWN · 2024-09-12T14:31:50Z

An unrelated error in https://github.com/cms-sw/cmssw/blob/master/RecoLuminosity/LumiDB/scripts/pileupCalc.py we get is the following:

$ pileupCalc.py -i /afs/cern.ch/user/e/emartinv/public/CMSSW_14_1_0_pre4/src/TauFW/PicoProducer/data/json/2024/Cert_Collisions2024_378981_385012_Golden.json --inputLumiJSON /afs/cern.ch/user/e/emartinv/public/CMSSW_14_1_0_pre4/src/TauFW/PicoProducer/data/json/2024/pileup_latest.txt --calcMode true --maxPileupBin 100 --numPileupBins 100 Data_PileUp_2024.root

[...]

Traceback (most recent call last):
  File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/pileupCalc.py", line 189, in <module>
    fillPileupHistogram(lumiInfo, options.calcMode,
  File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/pileupCalc.py", line 87, in fillPileupHistogram
    obs = binning.find(AveNumInt)
  File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/pileupCalc.py", line 65, in find
    return np.floor((x-self.xMin)*self.num/(self.xMax-self.xMin)).astype(np.int)
  File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/external/py3-numpy/1.24.3-ac08ea497df571aa0b37dc29bb7a2045/lib/python3.9/site-packages/numpy/__init__.py", line 305, in __getattr__
    raise AttributeError(__former_attrs__[attr])
AttributeError: module 'numpy' has no attribute 'int'.
`np.int` was a deprecated alias for the builtin `int`. To avoid this error in existing code, use `int` by itself. Doing this will not modify any behavior and is safe. When replacing `np.int`, you may wish to use e.g. `np.int64` or `np.int32` to specify the precision. If you wish to review your current use, check the release note link for additional information.
The aliases was originally deprecated in NumPy 1.20; for more details and guidance see the original release note at:
    https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations

from

cmssw/RecoLuminosity/LumiDB/scripts/pileupCalc.py

Lines 64 to 65 in c312b85

    
           def find(self, x): 
        
               return np.floor((x-self.xMin)*self.num/(self.xMax-self.xMin)).astype(np.int)

Shall I add a fix to this PR?

mmusich · 2024-09-12T14:34:35Z

Shall I add a fix to this PR?

that's what's discussed at #45742.
There was no input from LUMI so far, but I guess it would be nice to fix...

perrotta · 2024-09-12T15:25:54Z

Shall I add a fix to this PR?

Yes, please

perrotta · 2024-09-12T15:37:18Z

please test (It looks like PU above 100 was never attained in the tests before...)

After fixing the typo locally, we only see it happen four times in this Collisions24_13p6TeV_378981_385514_DCSOnly_TkPx.json file:
mean number of pileup events > 100 for run 384264, lum 862 : m 11721745844.106108 l 3072785342557351.500000
mean number of pileup events > 100 for run 384383, lum 213 : m 295925365174.010925 l 77575058928175920.000000
mean number of pileup events > 100 for run 384565, lum 642 : m 646223.555834 l 169403627820.420685
mean number of pileup events > 100 for run 385355, lum 913 : m 1230240082.388803 l 322500056157730.437500
Is this to be expected? I do not see any peak luminosities jumping out in 2024 compared to previous years in the summary plots?

A mean pileup as large as 295925365174 does not look healthy to me...

mmusich · 2024-09-12T15:45:46Z

the discussion here might or might not be related...

cmsbuild · 2024-09-12T16:28:15Z

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c9d045/41483/summary.html
COMMIT: 41a4450
CMSSW: CMSSW_14_2_X_2024-09-11-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45989/41483/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c9d045/41483/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c9d045/41483/git-merge-result

Comparison Summary

Summary:

You potentially removed 175 lines from the logs
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3331158
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3331133
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

Fixes ``` Traceback (most recent call last): File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/pileupCalc.py", line 189, in <module> fillPileupHistogram(lumiInfo, options.calcMode, File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/pileupCalc.py", line 87, in fillPileupHistogram obs = binning.find(AveNumInt) File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/pileupCalc.py", line 65, in find return np.floor((x-self.xMin)*self.num/(self.xMax-self.xMin)).astype(np.int) File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/external/py3-numpy/1.24.3-ac08ea497df571aa0b37dc29bb7a2045/lib/python3.9/site-packages/numpy/__init__.py", line 305, in __getattr__ raise AttributeError(__former_attrs__[attr]) AttributeError: module 'numpy' has no attribute 'int'. `np.int` was a deprecated alias for the builtin `int`. To avoid this error in existing code, use `int` by itself. Doing this will not modify any behavior and is safe. When replacing `np.int`, you may wish to use e.g. `np.int64` or `np.int32` to specify the precision. If you wish to review your current use, check the release note link for additional information. The aliases was originally deprecated in NumPy 1.20; for more details and guidance see the original release note at: https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations ```

cmsbuild · 2024-09-12T16:42:02Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45989/41763

cmsbuild · 2024-09-12T16:42:21Z

Pull request #45989 was updated. @cmsbuild, @consuegs, @francescobrivio, @perrotta, @saumyaphor4252 can you please check and sign again.

perrotta · 2024-09-12T17:35:52Z

please test

cmsbuild · 2024-09-12T20:52:17Z

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c9d045/41493/summary.html
COMMIT: 9ad24f8
CMSSW: CMSSW_14_2_X_2024-09-12-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45989/41493/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

@mandrenguyen Add 2024 HI eras to reco processing configuation #45981

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c9d045/41493/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c9d045/41493/git-merge-result

Comparison Summary

Summary:

You potentially removed 2 lines from the logs
Reco comparison results: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3331158
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3331138
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

perrotta · 2024-09-13T06:41:18Z

+db

The fix in the code is deserved
Still,I think one should investigate why bunch_del_lumy as stored in L90 can be such high, which results in an unrealistically large average pileup for the lumi section

cmsbuild · 2024-09-13T06:41:43Z

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

mandrenguyen · 2024-09-13T07:36:24Z

+1

IzaakWN · 2024-09-13T15:00:33Z

Still,I think one should investigate why bunch_del_lumy as stored in L90 can be such high, which results in an unrealistically large average pileup for the lumi section

This is not really the forum for this, so if you like I can post to the CMS Talk thread @mmusich referenced in case other stumble on these errors?

Still, in case it's useful, the error above were reported to me by Elvira (a TauPOG colleague). I managed to replicate it as follows (see "LumiRecommendationsRun3" TWiki):

$ wget https://cms-service-dqmdc.web.cern.ch/CAF/certification/Collisions24/DCSOnly_JSONS/dailyDCSOnlyJSON/Collisions24_13p6TeV_378981_385514_DCSOnly_TkPx.json

$ export BRILWS_VERSION=3.7.4
$ source /cvmfs/cms-bril.cern.ch/cms-lumi-pog/brilws-docker/brilws-env
$ brilcalc lumi --xing -i Collisions24_13p6TeV_378981_385514_DCSOnly_TkPx.json -b "STABLE BEAMS" --normtag /cvmfs/cms-bril.cern.ch/cms-lumi-pog/Normtags/normtag_BRIL.json --xingTr 0.1 -o /eos/user/i/ineuteli/lumi_DCSONLY.csv --datatag online

$ cp -v /cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre4/bin/el9_amd64_gcc12/makePileupJSON.py makePileupJSON.py
$ sed -i 's/runNumber/run/g' makePileupJSON.py
$ makePileupJSON.py /eos/user/i/ineuteli/lumi_DCSONLY.csv pileup_JSON.txt

Note brilcalc gave a bunch of warnings (the python3 iteritems warnings seem to have been fixed already in cms-bril/brilws:brilws/cli/brilcalc_main.py):

/home/bril/.local/lib/python3.10/site-packages/brilws/cli/brilcalc_main.py:644: FutureWarning: iteritems is deprecated and will be removed in a future version. Use .items instead.
  for r,l in rs.iteritems():
/home/bril/.local/lib/python3.10/site-packages/brilws/api.py:1211: SAWarning: Dialect oracle:frontier will not make use of SQL compilation caching as it does not set the 'supports_statement_cache' attribute to ``True``.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Dialect maintainers should seek to set this attribute to True after appropriate development and testing for SQLAlchemy 1.4 caching support.   Alternatively, this attribute may be set to False which will disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  return engine.dialect.has_table(engine.connect(),tablename,schema=schemaname)
/home/bril/.local/lib/python3.10/site-packages/brilws/api.py:1133: FutureWarning: iteritems is deprecated and will be removed in a future version. Use .items instead.
  for run,lsrange in inputSeries.iteritems():

Warning: problems found in merging -i and --normtag selections:
  runs [385422, 385423, 385424, 385437, 385443, 385444, 385447, 385474, 385478, 385479, 385480, 385481, 385483, 385484, 385511, 385512, 385513, 385514] are not covered by normtag
  in run 379774 [[28, 149], [151, 186]] is not a superset of [[29, 213]]
  in run 379866 [[25, 1372]] is not a superset of [[26, 1375]]
  in run 381113 [[59, 211], [220, 337]] is not a superset of [[60, 332]]
  in run 381309 [[31, 456]] is not a superset of [[32, 457]]
  in run 381417 [[34, 1822]] is not a superset of [[35, 1824]]
  in run 381443 [[31, 1550]] is not a superset of [[33, 1571]]
  in run 382568 [[29, 435]] is not a superset of [[30, 437]]
  in run 384579 [[31, 570]] is not a superset of [[33, 572]]
  in run 384614 [[33, 1289]] is not a superset of [[35, 1290]]
  in run 384981 [[28, 2132]] is not a superset of [[30, 2137]]
  in run 385012 [[47, 323]] is not a superset of [[49, 324]]
/home/bril/.local/lib/python3.10/site-packages/brilws/cli/brilcalc_main.py:752: FutureWarning: iteritems is deprecated and will be removed in a future version. Use .items instead.
  for k,v in rselect.iteritems():

And makePileupJSON.py gave

Processing /eos/user/i/ineuteli/lumi_DCSONLY_online.csv with all BX
mean number of pileup events > 100 for run 384264, lum 862 : m 11721745844.106108 l 3072785342557351.500000
mean number of pileup events > 100 for run 384383, lum 213 : m 295925365174.010925 l 77575058928175920.000000
mean number of pileup events > 100 for run 384565, lum 642 : m 646223.555834 l 169403627820.420685
mean number of pileup events > 100 for run 385355, lum 913 : m 1230240082.388803 l 322500056157730.437500
Output written to pileup_JSON_online.txt

Elvira also tried with this golden JSON, but only Run 385355 was not included.

Typo: runNumber → run

41a4450

cmsbuild added this to the CMSSW_14_2_X milestone Sep 12, 2024

cmsbuild added db-pending pending-signatures tests-pending orp-pending code-checks-pending labels Sep 12, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 12, 2024

cmsbuild added tests-started and removed tests-pending labels Sep 12, 2024

cmsbuild added tests-approved and removed tests-started labels Sep 12, 2024

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Sep 12, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 12, 2024

cmsbuild added tests-started and removed tests-pending labels Sep 12, 2024

cmsbuild added tests-approved and removed tests-started labels Sep 12, 2024

cmsbuild added fully-signed db-approved and removed db-pending pending-signatures labels Sep 13, 2024

cmsbuild added orp-approved and removed orp-pending labels Sep 13, 2024

cmsbuild merged commit e2a1fa1 into cms-sw:master Sep 13, 2024
11 checks passed

IzaakWN deleted the patch-1 branch September 13, 2024 15:01

This was referenced Sep 13, 2024

Refactoring of TICL EnergyRegression and PID model #45821

Merged

Fix a compilation warning in TracksterLinkingbySkeletons in DBG_X #45999

Merged

mmusich mentioned this pull request Sep 14, 2024

pileupCalc.py error #45742

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix small typo in `makePileupJSON.py`: `runNumber` → `run` #45989

Fix small typo in `makePileupJSON.py`: `runNumber` → `run` #45989

IzaakWN commented Sep 12, 2024 •

edited

Loading

cmsbuild commented Sep 12, 2024 •

edited

Loading

cmsbuild commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

perrotta commented Sep 12, 2024

IzaakWN commented Sep 12, 2024 •

edited

Loading

IzaakWN commented Sep 12, 2024

mmusich commented Sep 12, 2024

perrotta commented Sep 12, 2024

perrotta commented Sep 12, 2024

mmusich commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

perrotta commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

perrotta commented Sep 13, 2024

cmsbuild commented Sep 13, 2024

mandrenguyen commented Sep 13, 2024

IzaakWN commented Sep 13, 2024 •

edited

Loading

	for line in csv_input:
	if line[0] == '#':
	continue # skip comment lines

	pieces = sepRE.split(line.strip())

	if len(pieces) < 15:
	# The most likely cause of this is that we're using a csv file without the bunch data, so might as well
	# just give up now.
	raise RuntimeError("Not enough fields in input line; maybe you forgot to include --xing in your brilcalc command?\n"+line)
	try:
	run = int(pieces[0])
	lumi_section = int(pieces[2])
	beam_energy = float(pieces[10])
	#tot_del_lumi = float(pieces[11])
	#tot_rec_lumi = float(pieces[12])
	luminometer = pieces[14]

	# second loop to get (weighted) RMS
	for bxid, bunch_del_lumi, bunch_rec_lumi in xing_lumi_array:
	mean_pileup = bunch_del_lumi * parameters.orbitLength / parameters.lumiSectionLength
	if mean_pileup > 100:
	print("mean number of pileup events > 100 for run %d, lum %d : m %f l %f" % \
	(runNumber, lumi_section, mean_pileup, bunch_del_lumi))
	#print "mean number of pileup events for lum %d: m %f idx %d l %f" % (lumi_section, mean_pileup, bxid, bunch_rec_lumi)

Fix small typo in makePileupJSON.py: runNumber → run #45989

Fix small typo in makePileupJSON.py: runNumber → run #45989

Conversation

IzaakWN commented Sep 12, 2024 • edited Loading

cmsbuild commented Sep 12, 2024 • edited Loading

cmsbuild commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

perrotta commented Sep 12, 2024

IzaakWN commented Sep 12, 2024 • edited Loading

IzaakWN commented Sep 12, 2024

mmusich commented Sep 12, 2024

perrotta commented Sep 12, 2024

perrotta commented Sep 12, 2024

mmusich commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

Comparison Summary

cmsbuild commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

perrotta commented Sep 12, 2024

cmsbuild commented Sep 12, 2024

Comparison Summary

perrotta commented Sep 13, 2024

cmsbuild commented Sep 13, 2024

mandrenguyen commented Sep 13, 2024

IzaakWN commented Sep 13, 2024 • edited Loading

Fix small typo in `makePileupJSON.py`: `runNumber` → `run` #45989

Fix small typo in `makePileupJSON.py`: `runNumber` → `run` #45989

IzaakWN commented Sep 12, 2024 •

edited

Loading

cmsbuild commented Sep 12, 2024 •

edited

Loading

IzaakWN commented Sep 12, 2024 •

edited

Loading

IzaakWN commented Sep 13, 2024 •

edited

Loading