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

Run3-gex49 Correct the spellings in the scripts in CondTools/Geometry #32745

Merged
merged 2 commits into from Feb 3, 2021

Conversation

bsunanda
Copy link
Contributor

PR description:

Correct the spellings in the scripts in CondTools/Geometry

PR validation:

Tested using runTheMatrix test workflows

if this PR is a backport please specify the original PR and why you need to backport that PR:

Nothing special

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32745/20897

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master.

It involves the following packages:

CondTools/Geometry

@ggovi, @cmsbuild can you please review it and eventually sign? Thanks.
@mmusich this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@bsunanda
Copy link
Contributor Author

@cvuosalo Please check this

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32745/20899

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

Pull request #32745 was updated. @ggovi, @cmsbuild can you please check and sign again.

@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@cvuosalo
Copy link
Contributor

@bsunanda The PR description says these scripts were tested with runTheMatrix.py. That test does not touch these scripts. They have to be run individually. For example:

cd  CondTools/Geometry/test
/bin/cp writehelpers/* .
./createExtended2017Plan1Payloads.sh 113YV99

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-535b11/12551/summary.html
COMMIT: 4d259b6
CMSSW: CMSSW_11_3_X_2021-01-26-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32745/12551/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1096 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2716596
  • DQMHistoTests: Total failures: 3611
  • DQMHistoTests: Total nulls: 19
  • DQMHistoTests: Total successes: 2712944
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -45.703 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 140.53 ): -44.531 KiB Hcal/DigiRunHarvesting
  • DQMHistoSizes: changed ( 140.53 ): -1.172 KiB RPC/DCSInfo
  • Checked 156 log files, 37 edm output root files, 37 DQM output files

1 similar comment
@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-535b11/12551/summary.html
COMMIT: 4d259b6
CMSSW: CMSSW_11_3_X_2021-01-26-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32745/12551/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1096 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2716596
  • DQMHistoTests: Total failures: 3611
  • DQMHistoTests: Total nulls: 19
  • DQMHistoTests: Total successes: 2712944
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -45.703 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 140.53 ): -44.531 KiB Hcal/DigiRunHarvesting
  • DQMHistoSizes: changed ( 140.53 ): -1.172 KiB RPC/DCSInfo
  • Checked 156 log files, 37 edm output root files, 37 DQM output files

@cvuosalo
Copy link
Contributor

@bsunanda Search for "Perecent" in createExtended2017Plan1Payloads.sh. There are three misspellings that need to be corrected. Also, TrackerGeometricDetESModule has to be deleted from CondTools/Geometry/test/writehelpers/geometryExtended2017_writer.py and CondTools/Geometry/test/writehelpers/geometryExtended2018_writer.py.

createExtended2017Plan1Payloads.sh and createExtended2018Payloads.sh have not been used for quite some time, and they are currently non-functional, so you are opening a can of worms by touching them. They would be used to create new geometry DB payloads for Run 2, but there is currently no use case for this task.

You might want to drop createExtended2017Plan1Payloads.sh and createExtended2018Payloads.sh from this PR, and instead write an issue describing the problems with them that need to be fixed (what I mentioned above). If you keep them in this PR, you will need to debug and test them.

@cvuosalo
Copy link
Contributor

@bsunanda createExtended2021Payloads.sh looks good to me. It can stay in this PR. You should test it yourself to verify that it is working.

@ggovi
Copy link
Contributor

ggovi commented Feb 1, 2021

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 1, 2021

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
Copy link
Contributor

please test

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-535b11/12551/summary.html
COMMIT: 4d259b6
CMSSW: CMSSW_11_3_X_2021-01-26-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32745/12551/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

* No significant changes to the logs found

* Reco comparison results: 1096 differences found in the comparisons

* DQMHistoTests: Total files compared: 37

* DQMHistoTests: Total histograms compared: 2716596

* DQMHistoTests: Total failures: 3611

* DQMHistoTests: Total nulls: 19

* DQMHistoTests: Total successes: 2712944

* DQMHistoTests: Total skipped: 22

* DQMHistoTests: Total Missing objects: 0

* DQMHistoSizes: Histogram memory added: -45.703 KiB( 36 files compared)

* DQMHistoSizes: changed ( 140.53 ): -44.531 KiB      Hcal/DigiRunHarvesting

* DQMHistoSizes: changed ( 140.53 ): -1.172 KiB       RPC/DCSInfo

* Checked 156 log files, 37 edm output root files, 37 DQM output files

I'm surprised to see so many changes in 140.53 RunHI2011+RECOHID11+HARVESTDHI.
@bsunanda @cvuosalo @ggovi did you expect it? Perhaps it is uncorrelated to this PR therefore I started the test again

@cvuosalo
Copy link
Contributor

cvuosalo commented Feb 1, 2021

@silviodonato The scripts in this PR are not exercised by any PR test, so the PR comparison differences have nothing to do with this PR.
In fact, two scripts in this PR are not even working:

CondTools/Geometry/test/writehelpers/createExtended2017Plan1Payloads.sh
CondTools/Geometry/test/writehelpers/createExtended2018Payloads.sh

I don't think it is recommended practice to approve PRs with non-working scripts. At the very least, prominent "FIXME" comments should be added to the two scripts.

@silviodonato
Copy link
Contributor

please test

@silviodonato
Copy link
Contributor

@smuzaffar are the tests running? I see " cms/32745/slc7_amd64_gcc900/comparison Pending — Waiting for tests to start "

@smuzaffar
Copy link
Contributor

@silviodonato , yes tests are running, jenkins is still under heavy load after yesterday's network issues.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-535b11/12639/summary.html
COMMIT: 4d259b6
CMSSW: CMSSW_11_3_X_2021-02-01-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/32745/12639/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: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2752926
  • DQMHistoTests: Total failures: 5
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2752899
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 156 log files, 37 edm output root files, 37 DQM output files

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 18bc30c into cms-sw:master Feb 3, 2021
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