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

Async global end LuminosityBlock transition #20327

Merged
merged 4 commits into from
Sep 6, 2017

Conversation

Dr15Jones
Copy link
Contributor

The global end LuminosityBlock transition was supposed to be converted to asynchronous at the same time end Run was changed. This corrects that oversight. This also allowed removing many synchronous methods which were replaced with equivalent asynchronous implementations.

This was accidentally left off when the global end run was changed to asynchronous.
Removed methods which now have asynchronous versions and the synchronous versions are not used anymore.
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

FWCore/Framework
FWCore/Integration

@cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@Dr15Jones
Copy link
Contributor Author

please test

@Dr15Jones
Copy link
Contributor Author

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 30, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22605/console Started: 2017/08/31 03:20

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2)

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20327/404

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-20327/404/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/PR-20327/404/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly (this will soon be required for PRs to be considered)

@Dr15Jones
Copy link
Contributor Author

The unit test failure has nothing to do with this pull request.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20327/22623/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2656041
  • DQMHistoTests: Total failures: 217
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2655635
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 14 edm output root files, 26 DQM output files

@Dr15Jones
Copy link
Contributor Author

Please test
Although the previous test only failed because of unrelated reasons

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22642/console Started: 2017/09/01 14:22

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2017

-1

Tested at: d3907ad

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
94fab3e
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20327/22642/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20327/22642/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20327/22642/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test testRecoMETMETProducers had ERRORS

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
94fab3e
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20327/22642/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20327/22642/git-merge-result

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2017

Comparison job queued.

@Dr15Jones
Copy link
Contributor Author

@smuzaffar the unit test failure is not cause by this pull request. It appears to be a race condition in the testing sytem. The error is

01-Sep-2017 16:56:32 CEST  Initiating request to open file file:ttbarForMetTests.root
----- Begin Fatal Exception 01-Sep-2017 16:56:32 CEST-----------------------
An exception of category 'FileOpenError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing input source of type PoolSource
   [2] Calling RootFileSequenceBase::initTheFile()
   [3] Calling StorageFactory::open()
   [4] Calling File::sysopen()
Exception Message:
Failed to open the file 'ttbarForMetTests.root'
   Additional Info:
      [a] Input file file:ttbarForMetTests.root could not be opened.
      [b] open() failed with system error 'No such file or directory' (error code 2)
----- End Fatal Exception -------------------------------------------------
+ die 'Failure using recoMET_genMet_cfg.py' 84
+ echo Failure using recoMET_genMet_cfg.py: status 84
Failure using recoMET_genMet_cfg.py: status 84

but the file ttbarForMetTests.root is generated in the same shell script that runs the test:
https://github.com/cms-sw/cmssw/blob/CMSSW_9_3_X/RecoMET/METProducers/test/runtests.sh

The file in question was even successfully used by several tests run in the same shell script

+ cmsRun /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_3_X_2017-08-31-2300/src/RecoMET/METProducers/test/recoMET_caloMet_cfg.py
01-Sep-2017 16:50:17 CEST  Initiating request to open file file:ttbarForMetTests.root
01-Sep-2017 16:50:19 CEST  Successfully opened file file:ttbarForMetTests.root
Begin processing the 1st record. Run 1, Event 1, LumiSection 1 at 01-Sep-2017 16:50:49.343 CEST
01-Sep-2017 16:50:52 CEST  Closed file file:ttbarForMetTests.root

and

+ cmsRun /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_3_X_2017-08-31-2300/src/RecoMET/METProducers/test/recoMET_tcMet_cfg.py
01-Sep-2017 16:50:58 CEST  Initiating request to open file file:ttbarForMetTests.root
01-Sep-2017 16:51:00 CEST  Successfully opened file file:ttbarForMetTests.root
Begin processing the 1st record. Run 1, Event 1, LumiSection 1 at 01-Sep-2017 16:51:07.395 CEST
01-Sep-2017 16:56:24 CEST  Closed file file:ttbarForMetTests.root

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2017

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20327/22642/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2656041
  • DQMHistoTests: Total failures: 210
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2655642
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 14 edm output root files, 26 DQM output files

@Dr15Jones
Copy link
Contributor Author

@smuzaffar @davidlange6 There are several tests which do rm *.root which if run at the same time as testRecoMETMETProducers would cause the failure happening in this pull request.

cdj@cdj> git grep 'rm ' | grep '\*.root' | grep sh | grep test
Alignment/APEEstimation/test/cfgTemplate/createStep2.bash:  rm ${ROOTFILEBASE}/workingArea/*.root
DQMServices/Components/test/run_fastHadd_tests.sh:    rm -fr *.root
DQMServices/Core/python/test/test_DQMMultiThread.sh:  rm -fr *.root
FastSimulation/MaterialEffects/test/getNIFiles.sh:rm *.root
FastSimulation/MaterialEffects/test/getNIFiles_py.sh:rm *.root
HLTrigger/Configuration/test/runIntegration.csh:    rm -f  ${name}/*.root

@smuzaffar
Copy link
Contributor

good catch @Dr15Jones , yes this explains why these root files disappear.
I can update build rule to create separate unittest-name directory to starts the test from. That should avoid these type of deletions. Some test might need fixing if they assume that they start from $CMSSW_BASE
OR fix these above mention tests to only delete those root files which they write.

@Dr15Jones
Copy link
Contributor Author

@davidlange6 do you intend on including this in 9_3 or waiting until 9_4?

@Dr15Jones
Copy link
Contributor Author

@davidlange6 the unit test failures are not do to this pull request.

@smuzaffar smuzaffar modified the milestones: CMSSW_9_4_X, CMSSW_9_3_X Sep 6, 2017
@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 90a4be0 into cms-sw:master Sep 6, 2017
@Dr15Jones Dr15Jones deleted the asyncGlobalEndLumi branch September 8, 2017 18:15
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.

4 participants