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

Test if yoda_pyroot.patch is still necessary (after dropping Py2) #7355

Merged
merged 1 commit into from Oct 4, 2021

Conversation

iarspider
Copy link
Contributor

No description provided.

@iarspider
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 4, 2021

A new Pull Request was created by @iarspider for branch IB/CMSSW_12_1_X/master.

@smuzaffar, @iarspider, @mrodozov can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.
cms-bot commands are listed here

@davidlange6
Copy link
Contributor

this patch has nothing to do with python2 vs 3.. I'm not sure that the Yoda python interface is tested so its not obvious that a positive test result is sufficient to conclude that this patch is not needed...

@smuzaffar
Copy link
Contributor

To support py2/3 in root, here was what was suggested by root team

As a follow up of our discussion a couple of weeks ago, we've been
working with Massimiliano on this PR (not merged yet):

https://github.com/root-project/root/pull/5083

The highlights are:
- We will place all the libs (included the PyROOT ones) in the
ROOTSYS/lib directory, instead of using subdirectories for each Python
version. The PyROOT libraries will have the version number in the
library name, e.g. libcppyy2_7.so, to allow to build for both Python2
and Python3 at the same time. This solves the problem of having
duplicate library names when symlinking everything into the lib of CMSSW.
- We will only generate one libTPython.so. The reason is the PCMs: we
can't have libTPython2_7.so and libTPython3_6.so, plus their respective
PCMs, because all the PCMs are loaded by ROOT and we would have
duplicated symbols.
- To solve the Yoda case, we propose that Yoda does not link anymore
against libTPython, but against libcppyy. libcppyy contains the
functionality that Yoda needs, it is just a matter of including a
different header and slightly changing the function names that are being
invoked (we can give you more details about this). libcppyy will have
two versions (e.g. libcppyy2_7 and libcppyy3_6), so Yoda will be able to
be used from both Pythons. 

I think without this patch yoda was even failing to build

@smuzaffar
Copy link
Contributor

please test for CMSSW_12_1_ROOT6_X

@davidlange6
Copy link
Contributor

davidlange6 commented Oct 4, 2021 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 4, 2021

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c40715/19357/summary.html
COMMIT: cb4caec
CMSSW: CMSSW_12_1_X_2021-10-03-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/7355/19357/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

----- Begin Fatal Exception 04-Oct-2021 12:51:39 CEST-----------------------
An exception of category 'TrackingTools/PatternTools' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 9 stream: 0
   [1] Running path 'HLT_TrkMu6NoFiltersNoVtx_v1'
   [2] Calling method for module CkfTrackCandidateMaker/'hltIterL3OITrackCandidatesNoVtx'
Exception Message:
Trajectory::check() - information requested from empty Trajectory 
----- End Fatal Exception -------------------------------------------------

@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 4, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c40715/19374/summary.html
COMMIT: cb4caec
CMSSW: CMSSW_12_1_X_2021-10-04-1300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/7355/19374/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: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 40
  • DQMHistoTests: Total histograms compared: 3219394
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3219366
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
  • Checked 169 log files, 37 edm output root files, 40 DQM output files
  • TriggerResults: no differences found

@smuzaffar
Copy link
Contributor

+externals

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 4, 2021

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_12_1_X/master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@smuzaffar smuzaffar merged commit 6cc5839 into IB/CMSSW_12_1_X/master Oct 4, 2021
@smuzaffar smuzaffar deleted the drop_yoda_pyroot_patch branch October 4, 2021 19:22
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

4 participants