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

New PCL Alignment Threshold object in alignment consumers #38273

Merged
merged 2 commits into from Jun 13, 2022

Conversation

dmeuser
Copy link
Contributor

@dmeuser dmeuser commented Jun 7, 2022

PR description:

This PR is the second step towards using a higher granularity (HG) for the pixel alignment in the PCL, following #38195, where the new alignment threshold object was introduced. In this PR the consumer in the Alignment package are adapted to use the new threshold object.

In detail the following changes have been made:

  • Change from AlignPCLThresholds to AlignPCLThresholdsHG in the consumer code of the Alignment package
  • This PR updates the autoCond with GTs containing SiPixelAliThresholdsHG_offline_v0 and SiPixelAliThresholdsHG_express_v0

PR validation:

The PR was tested and validated using runTheMatrix -l 1001.0

Upcoming PRs:

This PR will be followed by one additional PR, which will introduces the changes needed to run and store the HG alignment

New GlobalTags:

The new GTs are

124X_dataRun2_v2
124X_dataRun2_relval_v2
124X_dataRun2_HEfail_v2

124X_dataRun3_v3
124X_dataRun3_relval_v3

124X_dataRun3_Express_frozen_v1
124X_dataRun2_PromptLike_HI_v1
124X_dataRun3_Prompt_frozen_v1
where the frozen GTs are the snapshotter versions of the newly created

124X_dataRun3_Express_v1
124X_dataRun3_Prompt_v1
The snapshot time is
2022-06-09 20:00:00 (UTC)

The diffs

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun2_v1/124X_dataRun2_v2

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun2_relval_v1/124X_dataRun2_relval_v2

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun2_HEfail_v1/124X_dataRun2_HEfail_v2

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun3_v2/124X_dataRun3_v3

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun3_relval_v2/124X_dataRun3_relval_v3

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/123X_dataRun3_Express_frozen_v4/124X_dataRun3_Express_frozen_v1

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/123X_dataRun2_PromptLike_HI_v3/124X_dataRun2_PromptLike_HI_v1

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/123X_dataRun3_Prompt_frozen_v4/124X_dataRun3_Prompt_frozen_v1

cc:
@mmusich, @connorpa, @antoniovagnerini, @consuegs

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38273/30424

  • This PR adds an extra 48KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2022

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

It involves the following packages:

  • Alignment/CommonAlignmentProducer (alca)
  • Alignment/MillePedeAlignmentAlgorithm (alca)

@cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please review it and eventually sign? Thanks.
@pakhotin, @mmusich, @adewit, @tocheng, @tlampen 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

@tvami
Copy link
Contributor

tvami commented Jun 7, 2022

type trk

@cmsbuild cmsbuild added the trk label Jun 7, 2022
@tvami
Copy link
Contributor

tvami commented Jun 7, 2022

Add code to supply the AlignPCLThresholdsHGRcd via ESSource

Ok, let's test this as it is now, and then we should provide this Rcd from the GT in a follow-up. This we can do after the L1T changes have been in the GT (so 2-3 more days)

@tvami
Copy link
Contributor

tvami commented Jun 7, 2022

test parameters:

  • workflows = 1001.0

@tvami
Copy link
Contributor

tvami commented Jun 7, 2022

@cmsbuild , please test

@mmusich
Copy link
Contributor

mmusich commented Jun 7, 2022

Ok, let's test this as it is now, and then we should provide this Rcd from the GT in a follow-up. This we can do after the L1T changes have been in the GT (so 2-3 more days)

No. A new pre-release is needed for inclusion in GT (the record is currently not recognized by the browser).

Screenshot from 2022-06-07 17-49-56

@tvami
Copy link
Contributor

tvami commented Jun 7, 2022

A new pre-release is needed for inclusion in GT (the record is currently not recognized by the browser)

I think that's not true. I can add it with admin rights, this is what I did
AlignPCLThresholdsHGRcd | AlignPCLThresholdsHG
in the record management.

Please confirm that's the correct matching.
If it is please upload the tag to Prod. The Prep DB is buggy so I couldnt do the same there (I'll follow-up with Andres abt the error)

@mmusich
Copy link
Contributor

mmusich commented Jun 7, 2022

I think that's not true.

this used to be the case in order to protect wrong requests of record on releases that don't support yet new records.
If the behaviour has been changed, this in an unwelcome feature that should be removed.

@malbouis
Copy link
Contributor

malbouis commented Jun 7, 2022

I think that's not true.

this used to be the case in order to protect wrong requests of record on releases that don't support yet new records. If the behaviour has been changed, this in an unwelcome feature that should be removed.

@mmusich , would you mind expanding on why this is an unwelcome feature?

@mmusich
Copy link
Contributor

mmusich commented Jun 7, 2022

would you mind expanding on why this is an unwelcome feature?

because it can lead the users (and the conveners unintentionally) to create broken global tags for the intended target queue.

@tvami
Copy link
Contributor

tvami commented Jun 7, 2022

OK the pre-release comes out today, so we can wait with this until tomorrow.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-edcbe3/25344/summary.html
COMMIT: 091e7d2
CMSSW: CMSSW_12_5_X_2022-06-07-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38273/25344/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3652078
  • DQMHistoTests: Total failures: 8
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3652048
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented Jun 9, 2022

@cms-sw/alca-l2 I see that the 12_5_0_pre2 build is ready.
Shall we wait that it becomes propagated to the CondDB tools, re-upload, make new GTs, etc, or can we just proceed with this as is and fix it later? This PR is a blocker for the further (and largest) new developments concerning the HG Alignment PCL.

@tvami
Copy link
Contributor

tvami commented Jun 9, 2022

Hi Marco, pre2 is already propagated to the conddb tools.
Screen Shot 2022-06-09 at 08 04 26

Please go ahead with uploading the tag to Prod, and we can cut new GTs.
Just a confirmation: this development is not meant to be for 12_3_X (just 12_4_X), correct?

@mmusich
Copy link
Contributor

mmusich commented Jun 9, 2022

Just a confirmation: this development is not meant to be for 12_3_X (just 12_4_X), correct?

it is meant for at least 12_4_X

@tvami
Copy link
Contributor

tvami commented Jun 9, 2022

it is meant for at least 12_4_X

Yes, so that's the grammar that confused me for #38297 (comment)

Ok, let's agree that this is for 12_4_X then

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38273/30539

  • This PR adds an extra 12KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

Pull request #38273 was updated. @cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Jun 13, 2022

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-edcbe3/25473/summary.html
COMMIT: 138bab0
CMSSW: CMSSW_12_5_X_2022-06-12-0000/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38273/25473/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3658678
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3658648
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@tvami
Copy link
Contributor

tvami commented Jun 13, 2022

+alca

This PR updates the autoCond with GTs containing SiPixelAliThresholdsHG_offline_v0 and SiPixelAliThresholdsHG_express_v0

The new GTs are

124X_dataRun2_v2
124X_dataRun2_relval_v2
124X_dataRun2_HEfail_v2

124X_dataRun3_v3
124X_dataRun3_relval_v3

124X_dataRun3_Express_frozen_v1
124X_dataRun2_PromptLike_HI_v1
124X_dataRun3_Prompt_frozen_v1
where the frozen GTs are the snapshotter versions of the newly created

124X_dataRun3_Express_v1
124X_dataRun3_Prompt_v1
The snapshot time is
2022-06-09 20:00:00 (UTC)

The diffs

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun2_v1/124X_dataRun2_v2

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun2_relval_v1/124X_dataRun2_relval_v2

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun2_HEfail_v1/124X_dataRun2_HEfail_v2

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun3_v2/124X_dataRun3_v3

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun3_relval_v2/124X_dataRun3_relval_v3

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/123X_dataRun3_Express_frozen_v4/124X_dataRun3_Express_frozen_v1

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/123X_dataRun2_PromptLike_HI_v3/124X_dataRun2_PromptLike_HI_v1

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/123X_dataRun3_Prompt_frozen_v4/124X_dataRun3_Prompt_frozen_v1

@cmsbuild
Copy link
Contributor

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

@dmeuser
Copy link
Contributor Author

dmeuser commented Jun 13, 2022

* @dmeuser please update the PR describtion with the following

@tvami done

@qliphy
Copy link
Contributor

qliphy commented Jun 13, 2022

+1

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.

None yet

6 participants