Shift from Boost Python to Pybind11 #34692

Purva-Chaudhari · 2021-07-29T11:18:51Z

PR description:

Shift from Boost Python to Pybind11 in CondCore Utilities

PR validation:

Passed on scram -b runtests (CondCore)

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

@davidlange6 @vgvassilev

cmsbuild · 2021-07-29T11:34:24Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24318

This PR adds an extra 32KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CondCore/SiPixelPlugins/test/testSiPixelPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/SiStripPlugins/test/testSiStripPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspector.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspectorModule.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/BuildFile.xml modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/Module_PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/src/PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24318/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24318/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-07-29T12:15:01Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24319

This PR adds an extra 36KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CondCore/SiPixelPlugins/test/testSiPixelPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/SiStripPlugins/test/testSiStripPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspector.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspectorModule.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/BuildFile.xml modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/Module_PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/src/PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672

cmsbuild · 2021-07-29T12:15:26Z

A new Pull Request was created by @Purva-Chaudhari for master.

It involves the following packages:

CondCore/SiPixelPlugins (db)
CondCore/SiStripPlugins (db)
CondCore/Utilities (db)

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

cms-bot commands are listed here

tvami · 2021-07-29T12:36:24Z

@cmsbuild , please test

mmusich · 2021-07-29T12:38:29Z

isn't this a (partial) duplicate of #34672 ?

davidlange6 · 2021-07-29T12:39:45Z

yes, working with @ggovi on this

mmusich · 2021-07-29T12:45:54Z

#34672 has been tested in a production environment aside from unit test, would be good that whatever final solution is used is tested in the same fashion (maybe you've already planned that)

cmsbuild · 2021-07-30T10:20:56Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24337

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CondCore/SiPixelPlugins/test/testSiPixelPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/SiStripPlugins/test/testSiStripPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspector.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspectorModule.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/BuildFile.xml modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/Module_PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/src/PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24337/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24337/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-07-30T10:49:53Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34692/24339

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CondCore/SiPixelPlugins/test/testSiPixelPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/SiStripPlugins/test/testSiStripPayloadInspector.cpp modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspector.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/interface/PayloadInspectorModule.h modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/BuildFile.xml modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/plugins/Module_PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672
- File CondCore/Utilities/src/PayloadInspector.cc modified in PR(s): Payload inspector framework moved to pybind11 #34672

cmsbuild · 2021-07-30T10:50:17Z

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

tvami · 2021-07-31T01:16:59Z

@cmsbuild , please test

cmsbuild · 2021-07-31T04:30:41Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e58e39/17380/summary.html
COMMIT: 9223cae
CMSSW: CMSSW_12_1_X_2021-07-30-0900/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34692/17380/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: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 2998564
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2998529
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

qliphy · 2021-08-10T02:09:27Z

@cms-sw/db-l2 Any comment?

ggovi · 2021-08-10T14:44:16Z

I'm expecting instructions from @davidlange6 on how to deal with this one. #34672 has been already validated in the web application.

qliphy · 2021-08-18T01:43:09Z

I'm expecting instructions from @davidlange6 on how to deal with this one. #34672 has been already validated in the web application.

kindly ping @ggovi @davidlange6

davidlange6 · 2021-08-18T17:34:23Z

sorry for the delay - I think its mostly the same in the end - this one diagonalizes the lexical_cast migration (partly done in @ggovi s pr) and follows the usual pybind11 conventions for declaring modules (functional interface rather than repeated m.def... ) but minor details.

qliphy · 2021-08-23T09:07:05Z

sorry for the delay - I think its mostly the same in the end - this one diagonalizes the lexical_cast migration (partly done in @ggovi s pr) and follows the usual pybind11 conventions for declaring modules (functional interface rather than repeated m.def... ) but minor details.

Thanks @davidlange6
As @ggovi mentioned: #34672 "has been already validated in the web application.", and #34672 also touches additional files than in this PR. Should we go ahead with #34672 and close this one?

smuzaffar · 2021-08-23T15:55:23Z

@qliphy , I would suggest to go ahead with #34672 which db has already tested in their web appilcation and close this

Purva-Chaudhari added 7 commits July 29, 2021 16:14

Update testSiPixelPayloadInspector.cpp

3d8b07a

Update testSiStripPayloadInspector.cpp

358735f

Update PayloadInspector.h

e7f6c85

Update PayloadInspectorModule.h

0b8f76a

Update Module_PayloadInspector.cc

346e333

Update getPayloadData.py

7b6d17f

Update PayloadInspector.cc

2db21b9

cmsbuild added this to the CMSSW_12_0_X milestone Jul 29, 2021

cmsbuild added code-checks-pending db-pending orp-pending pending-signatures tests-pending labels Jul 29, 2021

Update BuildFile.xml

cd301ec

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 29, 2021

applied suggested code checks

49e40a5

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 29, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 29, 2021

cmsbuild added tests-started and removed tests-pending labels Jul 29, 2021

Update PayloadInspector.cc

e18153f

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 30, 2021

Applied code check

9223cae

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 30, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 30, 2021

cmsbuild added tests-started and removed tests-pending labels Jul 31, 2021

cmsbuild added tests-approved and removed tests-started labels Jul 31, 2021

cmsbuild mentioned this pull request Aug 2, 2021

Fix params for Payload Inspector #34728

Merged

qliphy mentioned this pull request Aug 8, 2021

Payload inspector framework moved to pybind11 #34672

Merged

cmsbuild mentioned this pull request Aug 10, 2021

Replace boost::regex with std::regex #34826

Merged

cmsbuild mentioned this pull request Aug 18, 2021

Removed lexical_cast boost dependency in CondCore #34932

Merged

cmsbuild mentioned this pull request Aug 23, 2021

DQM Pixel Phase 1: Several new Efficiency trend plots #34384

Merged

qliphy closed this Aug 24, 2021

Purva-Chaudhari deleted the testbranch branch August 30, 2021 10:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shift from Boost Python to Pybind11 #34692

Shift from Boost Python to Pybind11 #34692

Purva-Chaudhari commented Jul 29, 2021

cmsbuild commented Jul 29, 2021

cmsbuild commented Jul 29, 2021

cmsbuild commented Jul 29, 2021

tvami commented Jul 29, 2021

mmusich commented Jul 29, 2021

davidlange6 commented Jul 29, 2021

mmusich commented Jul 29, 2021

cmsbuild commented Jul 30, 2021

cmsbuild commented Jul 30, 2021

cmsbuild commented Jul 30, 2021

tvami commented Jul 31, 2021

cmsbuild commented Jul 31, 2021

qliphy commented Aug 10, 2021

ggovi commented Aug 10, 2021

qliphy commented Aug 18, 2021

davidlange6 commented Aug 18, 2021

qliphy commented Aug 23, 2021

smuzaffar commented Aug 23, 2021

Shift from Boost Python to Pybind11 #34692

Shift from Boost Python to Pybind11 #34692

Conversation

Purva-Chaudhari commented Jul 29, 2021

PR description:

PR validation:

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

cmsbuild commented Jul 29, 2021

cmsbuild commented Jul 29, 2021

cmsbuild commented Jul 29, 2021

tvami commented Jul 29, 2021

mmusich commented Jul 29, 2021

davidlange6 commented Jul 29, 2021

mmusich commented Jul 29, 2021

cmsbuild commented Jul 30, 2021

cmsbuild commented Jul 30, 2021

cmsbuild commented Jul 30, 2021

tvami commented Jul 31, 2021

cmsbuild commented Jul 31, 2021

Comparison Summary

qliphy commented Aug 10, 2021

ggovi commented Aug 10, 2021

qliphy commented Aug 18, 2021

davidlange6 commented Aug 18, 2021

qliphy commented Aug 23, 2021

smuzaffar commented Aug 23, 2021