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

[PyROOT] Update cppyy to a recent version #205

Closed

Conversation

smuzaffar
Copy link

No description provided.

@smuzaffar
Copy link
Author

test parameters:

  • full_cmssw = true

@smuzaffar
Copy link
Author

please test for CMSSW_14_1_ROOT6_X

@cmsbuild
Copy link

A new Pull Request was created by @smuzaffar for branch cms/master/4279cb9707.

@iarspider, @smuzaffar, @aandvalenzuela can you please review it and eventually sign? Thanks.
@missirol this is something you requested to watch as well.
@sextonkennedy, @antoniovilela, @rappoccio you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link

cmsbuild commented Mar 15, 2024

cms-bot internal usage

@cmsbuild
Copy link

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-117c78/38180/summary.html
COMMIT: 98e5ecc
CMSSW: CMSSW_14_1_ROOT6_X_2024-03-14-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/root/205/38180/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-117c78/38180/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-117c78/38180/git-merge-result

Comparison Summary

Summary:

  • You potentially removed 173 lines from the logs
  • Reco comparison results: 911 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3288140
  • DQMHistoTests: Total failures: 9144
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3278976
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -1.092 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 12834.0,... ): -0.156 KiB HLT/Filters
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 7 / 46 workflows

@cmsbuild
Copy link

Pull request #205 was updated.

@smuzaffar
Copy link
Author

please test for CMSSW_14_1_ROOT6_X

@cmsbuild
Copy link

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-117c78/38194/summary.html
COMMIT: b3a666d
CMSSW: CMSSW_14_1_ROOT6_X_2024-03-15-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/root/205/38194/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-117c78/38194/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-117c78/38194/git-merge-result

Unit Tests

I found 2 errors in the following unit tests:

---> test testHeterogeneousCoreAlpakaTestModulesCPU had ERRORS
---> test testHeterogeneousCoreAlpakaTestWriteReadSerialSync had ERRORS

Comparison Summary

Summary:

This reverts commit d5efd70.

We are patching the automatic conversion to Python strings back in, so
it's not necessary to Pythonize a `__str__` funciton implementing it in
C++.

Also, the `hasattr(foo, "___cpp__str")` caused a *huge* performance it
in some cases, because looking up a non-existing attribute in cppyy can
be quite expensive. All base classes are crawled too, and that invokes
the interpreter and string manipulation.
The added script will be used to synchronize `cppyy` and `CPyCppyy`,
applying some patches that are necessary for ROOT.
This was done with the `sync-upstream` script, introduced in the last
commit.
Other than the `cppyy` Python library and the `CPyCppyy` CPython
extension, the `cppyy-backend` can't easily be synchronized with
upstream. The reason is that it depends both on patches to cling and to
ROOT meta. There is no bookkeeping on the patches to ROOT meta which is
complicating things, and the patches might also interfere with other
ROOT functionality.

A possible synchronization of ROOT meta is also not worth the effor for
another reason: it will be replaced by libInterOp in the future in the
context of cppyy and PyROOT.

Furthermore, synchronizing the backend would not result in fixing any
further reported ROOT issues.

Therefore, only minimal changes were made to the `cppyy-backend` in the
cppyy upgrade.
The reference count in Python is usually quite fragile to implementation
changes, so it's not too surprising that some counts change with the
`CPyCppyy` upgrade.
@cmsbuild
Copy link

Pull request #205 was updated.

@smuzaffar smuzaffar changed the base branch from cms/master/4279cb9707 to cms/master/d85c49a499 March 19, 2024 12:11
@smuzaffar
Copy link
Author

please test for CMSSW_14_1_ROOT6_X

@cmsbuild
Copy link

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-117c78/38259/summary.html
COMMIT: 9c4418a
CMSSW: CMSSW_14_1_ROOT6_X_2024-03-18-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/root/205/38259/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@hahnjo
Copy link

hahnjo commented Jun 27, 2024

root-project#14507 has long been merged, I believe this can be closed

@smuzaffar smuzaffar closed this Aug 1, 2024
@guitargeek guitargeek deleted the CPyCppyy-1.12.16 branch August 20, 2024 09:51
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