Skip to content

Conversation

@smuzaffar
Copy link
Contributor

@smuzaffar smuzaffar commented Dec 11, 2024

Looks like edm class checks silently ignore missing rootmap files and suggest a different checksum when rootmap files are not there. New tag makes sure that rootmap files are also moved to lib production store when pcm files are moved.

Looks like if rootmap files (which are generated (along with pcm) when we generate root dict source file) are missing then root suggests different checksum. For normal PR tests it works as rootmap files are read from the release area but for PR tests with full rebuild ( where we remove ref to release area) the rootmap files are missing and root suggests different checksum. A simple script ( copied from edm class check) [a] shows that if we have rootmap files then class checksum is same as we have in classes_def file but if we remove the rootmap files then it shows different checksum [b]

[a]

> cat get_class_checksum.py
#!/usr/bin/env python3
import sys, ROOT
name=sys.argv[1]
lib=sys.argv[2]
ROOT.PyConfig.DisableRootLogon = True
ROOT.gROOT.SetBatch(True)
ROOT.gROOT.ProcessLine(".autodict")
ROOT.gSystem.Load("lib%s.so" % lib)
ROOT.gROOT.ProcessLine("class checkclass {public: int f(char const* name) {TClass* cl = TClass::GetClass(name); bool b = false; cl->GetCheckSum(b); return (int)b;} };")
ROOT.gROOT.ProcessLine("checkclass checkTheClass;")
c = ROOT.TClass.GetClass(name)
temp = "checkTheClass.f(" + '"' + name + '"' + ");"
ROOT.gROOT.ProcessLine(temp)
print(c.GetCheckSum(),c.GetClassVersion())

[b]

  • DataFormats/CTPPSDetId/src/classes_def.xml
<class name="CTPPSDetId" ClassVersion="3">
    <version ClassVersion="3" checksum="2579437464"/>
</class>
  • content of lib/arch
DataFormatsCTPPSDetId_xr.rootmap    DataFormatsCommon_xr.rootmap
DataFormatsDetId_xr.rootmap    DataFormatsProvenance_xr.rootmap
libDataFormatsCTPPSDetId.so  libDataFormatsDetId.so
libFWCoreMessageLogger.so  libFWCoreReflection.so
DataFormatsCTPPSDetId_xr_rdict.pcm  DataFormatsCommon_xr_rdict.pcm
DataFormatsDetId_xr_rdict.pcm  DataFormatsProvenance_xr_rdict.pcm
libDataFormatsCommon.so      libDataFormatsProvenance.so
libFWCorePluginManager.so  libFWCoreUtilities.so
  • running script with rootmap files available
> get_class_checksum.py CTPPSDetId DataFormatsCTPPSDetId
2579437464 3
  • running script without rootmap files
> mv lib/el8_amd64_gcc12/*.rootmap backup/
> get_class_checksum.py CTPPSDetId DataFormatsCTPPSDetId
1554945820 3

@smuzaffar
Copy link
Contributor Author

test parameters:

  • full_cmssw = true

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smuzaffar for branch IB/CMSSW_15_0_X/master.

@aandvalenzuela, @iarspider, @smuzaffar can you please review it and eventually sign? Thanks.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 11, 2024

cms-bot internal usage

@makortel
Copy link
Contributor

A simple script ( copied from edm class check) [a] shows that if we have rootmap files then class checksum is same as we have in classes_def file but if we remove the rootmap files then it shows different checksum [b]

Interesting behavior. @pcanal Can you tell if this is an expected behavior from the ROOT side? (I could imagine in absence of the rootmap file ROOT could auto-parse the headers, and that leading to a different checksum)

@pcanal
Copy link

pcanal commented Dec 11, 2024

This is unexpected. The checksum resulting in parsing the header at run-time should be exactly the same than the checksum seen by rootcling/genreflex. Specifically the 2 environment are meant to see exactly the same shape/layout for the classes involved in I/O.

An alternative explanation may (or may not) that the presence or absence of rootmap file in a specific directory changes which library is auto-loaded for a given class.

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

Pull request #9568 was updated.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-603fb9/43394/summary.html
COMMIT: d22076e
CMSSW: CMSSW_15_0_X_2024-12-11-0800/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/9568/43394/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-603fb9/43394/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-603fb9/43394/git-merge-result

Unit Tests

I found 1 errors in the following unit tests:

---> test TestDQMOfflineConfigurationGotAll had ERRORS

Comparison Summary

Summary:

@smuzaffar
Copy link
Contributor Author

+externals

@smuzaffar smuzaffar merged commit f1b6865 into IB/CMSSW_15_0_X/master Dec 11, 2024
9 of 10 checks passed
@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_15_0_X/master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @antoniovilela, @rappoccio, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)

@smuzaffar smuzaffar deleted the smuzaffar-patch-8 branch December 18, 2024 14:35
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.

5 participants