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

Update to remove per bx roc data - 14_0_X #45369

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

perrotta
Copy link
Contributor

@perrotta perrotta commented Jul 3, 2024

The modules affected:
Calibration/LumiAlCaRecoProducers
DataFormats/Luminosity

Minor change to the structure of per roc data. Effectively removes per bx granularity (hence reducing the array size by 3563), to resolve the memory usage issue raised in #45306

backport of #45348

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2024

A new Pull Request was created by @perrotta for CMSSW_14_0_X.

It involves the following packages:

  • DataFormats/Luminosity (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks.
@missirol, @mmusich, @rovere this is something you requested to watch as well.
@antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2024

cms-bot internal usage

@perrotta
Copy link
Contributor Author

perrotta commented Jul 3, 2024

please test

@perrotta
Copy link
Contributor Author

perrotta commented Jul 3, 2024

backport of #45348

@perrotta
Copy link
Contributor Author

perrotta commented Jul 3, 2024

type bug-fix

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2024

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5407a1/40202/summary.html
COMMIT: 15b0549
CMSSW: CMSSW_14_0_X_2024-07-03-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45369/40202/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@mandrenguyen
Copy link
Contributor

type lumi

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2024

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

@perrotta
Copy link
Contributor Author

perrotta commented Jul 4, 2024

urgent
(to be included in next 14_0_X)

@cmsbuild cmsbuild added the urgent label Jul 4, 2024
@antoniovilela
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 9331e90 into cms-sw:CMSSW_14_0_X Jul 4, 2024
10 checks passed
@perrotta perrotta deleted the removePerBxRocData_140X branch July 4, 2024 17:52
@perrotta
Copy link
Contributor Author

perrotta commented Aug 7, 2024

type changes-dataformats

@mmusich
Copy link
Contributor

mmusich commented Aug 7, 2024

type changes-dataformats

IIUC cms-sw/cms-bot#2245, this PR doesn't quality for this label as there is a change in the content but not in the class layout (@makortel please correct me if I a mistaken).

@perrotta
Copy link
Contributor Author

perrotta commented Aug 7, 2024

type changes-dataformats

IIUC cms-sw/cms-bot#2245, this PR doesn't quality for this label as there is a change in the content but not in the class layout (@makortel please correct me if I a mistaken).

I wanted to indicate that the size of the output data as ruled by the DataFormat has changed, which is what was being discussed today at the TSG meeting, Indeed the layout of the class wasn't changed, and therefore the label can be questionable: nonetheless, since this PR is already included in a full cmssw release since a while already, it will not be used to decide whether to make a patch or a full release. But I can remove it if people think it is not due.

@makortel
Copy link
Contributor

makortel commented Aug 7, 2024

type changes-dataformats

IIUC cms-sw/cms-bot#2245, this PR doesn't quality for this label as there is a change in the content but not in the class layout (@makortel please correct me if I a mistaken).

I wanted to indicate that the size of the output data as ruled by the DataFormat has changed, which is what was being discussed today at the TSG meeting, Indeed the layout of the class wasn't changed, and therefore the label can be questionable: nonetheless, since this PR is already included in a full cmssw release since a while already, it will not be used to decide whether to make a patch or a full release. But I can remove it if people think it is not due.

I think the changes-dataformats should include also changes that do not change the physical class layout, but change the interpretation of the data in an incompatible way. In other words, if data stored with/without this PR would fail to be read correctly without/with this PR, I'd say the changes-dataformats label would be justified.

We should probably try to formalize the intention (policy) of changes-dataformats and write it down e.g. in https://twiki.cern.ch/twiki/bin/viewauth/CMS/ReleaseSchedule .

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