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 for GTEntry_t hash function #14583

Conversation

ggovi
Copy link
Contributor

@ggovi ggovi commented May 20, 2016

This fix changes the implementation of the GTEntry_t hash function, used to extract the mapping Record ->Tag in a Global Tag. The new implementation allows to use the same tag for more record+labels in a GT, which was producing an error previously.
This is fully back-ward compatible ( while a new GT with more then one entry with the same tag will be readable only by this code )

@ggovi
Copy link
Contributor Author

ggovi commented May 20, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 20, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/13100/console

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ggovi for CMSSW_8_0_X.

It involves the following packages:

CondCore/CondDB

@ggovi, @cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @apfeiffer1 this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@cmsbuild
Copy link
Contributor

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

-1

Tested at: 3fbc499

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

I found follow errors while testing this PR

Failed tests: Build

  • Build:

I found an error when building:

 ^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-05-19-2300/src/MuonAnalysis/MomentumScaleCalibration/test/UnitTests/MasterTestMuScleFit.cpp:19:40: warning: /cvmfs/cms-ib.cern.ch/2016-21/slc6_amd64_gcc530/external/cppunit/1.12.1/include/cppunit/ui/text/TestRunner.h is shorter than expected
In file included from /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-05-19-2300/src/MuonAnalysis/MomentumScaleCalibration/test/UnitTests/MasterTestMuScleFit.cpp:26:0:
/cvmfs/cms-ib.cern.ch/2016-21/slc6_amd64_gcc530/external/cppunit/1.12.1/include/cppunit/BriefTestProgressListener.h:1:1: warning: null character(s) ignored
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-05-19-2300/src/MuonAnalysis/MomentumScaleCalibration/test/UnitTests/MasterTestMuScleFit.cpp: In function 'int main(int, char**)':
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-05-19-2300/src/MuonAnalysis/MomentumScaleCalibration/test/UnitTests/MasterTestMuScleFit.cpp:37:12: error: 'CppUnit::TextUi' has not been declared
   CppUnit::TextUi::TestRunner runner;
            ^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-05-19-2300/src/MuonAnalysis/MomentumScaleCalibration/test/UnitTests/MasterTestMuScleFit.cpp:39:3: error: 'runner' was not declared in this scope
   runner.addTest( registry.makeTest() );
   ^


@ggovi
Copy link
Contributor Author

ggovi commented May 20, 2016

@slava77 @Degano @smuzaffar
is this build broken?

@slava77
Copy link
Contributor

slava77 commented May 20, 2016

@cmsbuild please test
in case it was a glitch

CMSSW_8_0_X_2016-05-19-2300 used in the tests looks OK in https://cmssdt.cern.ch/SDT/html/showIB.html

The build error started with

warning: /cvmfs/cms-ib.cern.ch/2016-21/slc6_amd64_gcc530/external/cppunit/1.12.1/include/cppunit/ui/text/TestRunner.h
 is shorter than expected

@smuzaffar could it be a problem with cvmfs distr or some node issue?

@cmsbuild
Copy link
Contributor

cmsbuild commented May 20, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/13103/console

@cmsbuild
Copy link
Contributor

-1

Tested at: 3fbc499

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

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test runtestTqafTopEventSelection had ERRORS

@ggovi
Copy link
Contributor Author

ggovi commented May 20, 2016

@slava77
the error looks still unrelated...

@slava77
Copy link
Contributor

slava77 commented May 20, 2016

@ggovi
at least it compiles now :-)

The unit test was fixed in 81X already
The error is
pat::Tau: the ID againstElectronVLooseMVA5 can't be found in this pat::Tau.
#14055
@smuzaffar maybe we can backport your fix?

@smuzaffar
Copy link
Contributor

@slava77 , I will make Pr for 80X

@cmsbuild
Copy link
Contributor

@ggovi
Copy link
Contributor Author

ggovi commented May 24, 2016

@smuzaffar
any news on this?

@smuzaffar
Copy link
Contributor

@ggovi , it is done here #14596 . Waiting for analysis and orp signatures.

For this PR, unit test failure is unrelated, so can be merged. Still needs db signature though.

@ggovi
Copy link
Contributor Author

ggovi commented May 25, 2016

@smuzaffar
ok, thanks!

@ggovi
Copy link
Contributor Author

ggovi commented May 25, 2016

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (but tests are reportedly failing). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlange6 davidlange6 merged commit 1720467 into cms-sw:CMSSW_8_0_X May 25, 2016
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

5 participants