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

remove GENREFLEX_ARGS flag #27655

Merged
merged 1 commit into from Aug 5, 2019
Merged

remove GENREFLEX_ARGS flag #27655

merged 1 commit into from Aug 5, 2019

Conversation

smuzaffar
Copy link
Contributor

PR description:

Cleanup GENREFLEX_ARGS="--" flag from BuildFiles. It was only dropping the --deep flag for genreflex. As --deep is not supported by genreflex/rootcling any more ( cms-sw/cmsdist@b9f88fe ) , we propose to cleanup our BuildFiles too.

PR validation:

local compilation was successful

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27655/11201

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 31, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1743/console Started: 2019/07/31 16:38

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for master.

It involves the following packages:

CondCore/CondDB
CondFormats/Alignment
CondFormats/BTauObjects
CondFormats/BeamSpotObjects
CondFormats/CSCObjects
CondFormats/Calibration
CondFormats/CastorObjects
CondFormats/Common
CondFormats/DQMObjects
CondFormats/DTObjects
CondFormats/EcalCorrections
CondFormats/GEMObjects
CondFormats/GeometryObjects
CondFormats/HIObjects
CondFormats/HLTObjects
CondFormats/HcalObjects
CondFormats/L1TObjects
CondFormats/Luminosity
CondFormats/MFObjects
CondFormats/OptAlignObjects
CondFormats/PCLConfig
CondFormats/RPCObjects
CondFormats/RecoMuonObjects
CondFormats/RunInfo
CondFormats/SiPhase2TrackerObjects
CondFormats/SiPixelObjects
DPGAnalysis/SiStripTools

@cmsbuild, @benkrikler, @kmaeshima, @schneiml, @fwyzard, @christopheralanwest, @andrius-k, @tocheng, @Martin-Grunewald, @rekovic, @franzoni, @jfernan2, @fioriNTU, @tlampen, @ggovi, @pohsun, @santocch can you please review it and eventually sign? Thanks.
@echabert, @emilbols, @pieterdavid, @argiro, @HeinerTholen, @thomreis, @threus, @mmusich, @seemasharmafnal, @venturia, @smoortga, @acaudron, @dildick, @tocheng, @VinInn, @ferencek, @jandrea, @ebrondol, @ptcox, @jshlee, @JyothsnaKomaragiri, @mverzett, @dkotlins, @andrzejnovak, @pvmulder this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 31, 2019

-1

Tested at: 04898a0

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

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test ExpressionEvaluatorUnitTest had ERRORS

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2019

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

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

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-6bcc32/22034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2026D46_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D46+RecoFullGlobal_2026D46+HARVESTFullGlobal_2026D46

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 2484130
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2483812
  • DQMHistoTests: Total skipped: 317
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 132 log files, 14 edm output root files, 32 DQM output files

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1758/console Started: 2019/08/01 08:50

@smuzaffar
Copy link
Contributor Author

@VinInn , recently we have noticed the unit test ExpressionEvaluatorUnitTest is failing during PR tests. Looking closely in the IB, I see that this is also failing in IB https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc700/CMSSW_11_0_X_2019-07-31-2300/unitTestLogs/CommonTools/Utils#/2-29

Can you please look in to it and provide a fix?

[a]

/tmp/VI_24e3320a7f9a4edc9e209c3d3e0edd03.cc:3:6: error: 'bool VI_24e3320a7f9a4edc9e209c3d3e0edd03::operator()(int, int)' marked 'override', but does not override
 bool operator()(int i, int j) override { return i<10&& j<5; }
      ^~~~~~~~
/tmp/VI_24e3320a7f9a4edc9e209c3d3e0edd03.cc: In function 'reco::genericExpression<bool, int, int>* factoryVI_24e3320a7f9a4edc9e209c3d3e0edd03()':
/tmp/VI_24e3320a7f9a4edc9e209c3d3e0edd03.cc:6:44: error: cannot declare variable 'local' to be of abstract type 'VI_24e3320a7f9a4edc9e209c3d3e0edd03'
 static VI_24e3320a7f9a4edc9e209c3d3e0edd03 local;
                                            ^~~~~
/tmp/VI_24e3320a7f9a4edc9e209c3d3e0edd03.cc:2:8: note:   because the following virtual functions are pure within 'VI_24e3320a7f9a4edc9e209c3d3e0edd03':
 struct VI_24e3320a7f9a4edc9e209c3d3e0edd03 final : public reco::genericExpression<bool, int, int>{
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/CommonTools/Utils/src/precompile.h:2:0:
/build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-07-31-1100/src/CommonTools/Utils/interface/ExpressionEvaluatorTemplates.h:14:17: note: 	Ret reco::genericExpression<Ret, Args>::operator()(Args ...) const [with Ret = bool; Args = {int, int}]
     virtual Ret operator()(Args...) const = 0;
                 ^~~~~~~~
dlerror /tmp/VI_24e3320a7f9a4edc9e209c3d3e0edd03.so: cannot open shared object file: No such file or directory 

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2019

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 2484130
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2483811
  • DQMHistoTests: Total skipped: 317
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 132 log files, 14 edm output root files, 32 DQM output files

@fioriNTU
Copy link
Contributor

fioriNTU commented Aug 2, 2019

+1

@smuzaffar
Copy link
Contributor Author

@fabiocos , can we get this in? This is just a technical change to cleanup BuildFiles. This flag in BuildFile is useless after the merge of cms-sw/cmsdist#5110

@fabiocos
Copy link
Contributor

fabiocos commented Aug 5, 2019

@smuzaffar this looks quite straightforward, I will by-pass the signatures. Just one question; you are dropping this flag from all the BuildFiles where it is present, what about

https://cmssdt.cern.ch/lxr/source/Utilities/ReleaseScripts/python/cmsCodeRules/config.py#0190

?

Do we still need GENREFLEX_ARGS here at this point?

@fabiocos
Copy link
Contributor

fabiocos commented Aug 5, 2019

+1

@fabiocos
Copy link
Contributor

fabiocos commented Aug 5, 2019

merge

@cmsbuild cmsbuild merged commit ea77689 into cms-sw:master Aug 5, 2019
@smuzaffar
Copy link
Contributor Author

@fabiocos , we need to keep https://cmssdt.cern.ch/lxr/source/Utilities/ReleaseScripts/python/cmsCodeRules/config.py#0190 . GENREFLEX_ARGS is valid BuildFile flag to provide extra command line arguments for genreflex and we want cmsCodeRule to still ignore it if it is found in BuildFile but <flags GENREFLEX_ARGS="--"/> is useless.

@santocch
Copy link

santocch commented Aug 6, 2019

+1

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Aug 8, 2019

@VinInn can you please look in to this #27655 (comment) ? #27723 will fix.

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