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

Fix pat::Muon checksum #36972

Merged
merged 1 commit into from
Feb 15, 2022
Merged

Fix pat::Muon checksum #36972

merged 1 commit into from
Feb 15, 2022

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Feb 15, 2022

PR description:

Should fix the build failure discussed in #36179 (comment).

PR validation:

None, edited online.

@tvami
Copy link
Contributor

tvami commented Feb 15, 2022

type bug-fix

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36972/28340

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

  • DataFormats/PatCandidates (reconstruction)

@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks.
@gpetruc, @gouskos, @rovere, @hatakeyamak 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

@jpata
Copy link
Contributor

jpata commented Feb 15, 2022

@cmsbuild please test

since in the original PR the test didn't catch this, I'm wondering what the test will show here

@tvami
Copy link
Contributor

tvami commented Feb 15, 2022

isnt there a way to test the full IB setup?

@makortel
Copy link
Contributor Author

urgent

@tvami
Copy link
Contributor

tvami commented Feb 15, 2022

test parameters:

  • full = true

@makortel
Copy link
Contributor Author

isnt there a way to test the full IB setup?

There is, but I guess it wouldn't "catch" it either. The error is in the build log of the original PR, it just didn't propagate for some reason to the test status

@perrotta
Copy link
Contributor

Naively looking at the error message: shouldn't class version be increased to 30?

@makortel
Copy link
Contributor Author

Naively looking at the error message: shouldn't class version be increased to 30?

In principle yes, but in this specific case we can reuse 29 because we have not produced any data with version 29 (and the original PR should have used this value for 29). If we were fixing the class version e.g. a week from now, I would have increased the class version, but now we're talking about fixing it for the IB following the one that introduced 29.

smuzaffar added a commit to cms-sw/cmsdist that referenced this pull request Feb 15, 2022
@smuzaffar
Copy link
Contributor

failure here are not related to this change. PR tests are using a working CMSW_12_3_X_2022-02-14-2300 IB + then changes which have been merged in master branch (but these changes require cmsdist updats too e.g. cms-sw/cmsdist#7626 ) .

@smuzaffar
Copy link
Contributor

test parameters:

@smuzaffar
Copy link
Contributor

please test

@perrotta
Copy link
Contributor

+1

  • Let have this merged for the next IB, even if tests haven't finished yet, and hope that it can be cleaner than the previous one.

@perrotta
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 12d0df8 into cms-sw:master Feb 15, 2022
@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2737d8/22442/summary.html
COMMIT: 0a1108d
CMSSW: CMSSW_12_3_X_2022-02-14-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36972/22442/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-2737d8/22442/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2737d8/22442/git-merge-result

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-2737d8/138.4_PromptCollisions+RunMinimumBias2021+ALCARECOPROMPTR3+HARVESTDPROMPTR3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-2737d8/138.5_ExpressCollisions+RunMinimumBias2021+TIER0EXPRUN3+ALCARECOEXPR3+HARVESTDEXPR3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-2737d8/139.001_RunMinimumBias2021+RunMinimumBias2021+HLTDR3_2021+RECODR3_MinBiasOffline+HARVESTD2021MB

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 8230 differences found in the comparisons
  • DQMHistoTests: Total files compared: 46
  • DQMHistoTests: Total histograms compared: 3757427
  • DQMHistoTests: Total failures: 104326
  • DQMHistoTests: Total nulls: 20455
  • DQMHistoTests: Total successes: 3632624
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -56760.095 KiB( 45 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 1.487 KiB L1TEMU/L1TdeStage2uGMT
  • DQMHistoSizes: changed ( 23234.0,... ): -7096.313 KiB HGCAL/HGCalValidator
  • Checked 193 log files, 42 edm output root files, 46 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

Pull request #36972 was updated. @simonepigazzini, @vlimant can you please check and sign again.

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2737d8/34792/summary.html
COMMIT: 0a1108d
CMSSW: CMSSW_13_3_X_2023-09-17-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36972/34792/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 2 lines from the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3348648
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3348626
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 214 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

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.

7 participants