-
Notifications
You must be signed in to change notification settings - Fork 0
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
MiniAOD RNTuple conversion #6
Comments
The list of members to migrate was produced using this very hacky script import ROOT
exc_cache = {}
def safe_members(tclass):
for mem in tclass.GetListOfDataMembers():
if not mem.IsPersistent():
continue
mclass = ROOT.TClass.GetClass(mem.GetTypeName())
if not mclass:
yield mem
continue
if not mclass.HasDefaultConstructor():
continue
if mclass.ClassProperty() & ROOT.EClassProperty.kClassIsAbstract:
continue
yield mem
def printIndent(indent: int, msg: str):
print(" "*indent + msg)
def findbad(classname, indent=0, member_name=""):
if classname in exc_cache:
# it seems that RNTuple slowly goes mad?
msg = exc_cache[classname]
else:
try:
model = ROOT.Experimental.RNTupleModel.Create()
model.MakeField[classname]("test")
return True
except ROOT.Experimental.RException as ex:
msg = str(ex).split("\n")[2]
exc_cache[classname] = msg
except TypeError as ex:
# some weird state switch?
msg = "(TypeError) " + str(ex).split("\n")[0]
except ValueError as ex:
printIndent(indent, f"ValueError for {classname} {member_name}, quitting")
raise ex
finally:
del model
printIndent(indent, f"In {classname} {member_name}:")
tclass = ROOT.TClass.GetClass(classname)
if not tclass:
printIndent(indent+4, f"{msg} (no dictionary to recurse)")
return False
# https://github.com/root-project/root/blob/v6-30-01/tree/ntuple/v7/src/RField.cxx#L351
if classname.startswith("vector<"):
findbad(classname[classname.find("<")+1:classname.rfind(">")], indent + 4, "")
elif classname.startswith("multimap<") or classname.startswith("map<"):
printIndent(indent+4, f"{msg} (cannot recurse maps safely?)")
elif classname.startswith("atomic<"):
printIndent(indent+4, f"{msg}")
else:
good = True
for mem in safe_members(tclass):
good &= findbad(mem.GetTypeName(), indent + 4, mem.GetName())
for base in tclass.GetListOfBases():
bclass = ROOT.TClass.GetClass(base.GetName())
for mem in safe_members(bclass):
good &= findbad(mem.GetTypeName(), indent + 4, mem.GetName() + f" (base {base.GetName()})")
if good:
printIndent(indent+4, f"{msg} (no children failed)")
return False
badproducts_mini = [
"edm::RangeMap<CSCDetId,edm::OwnVector<CSCSegment,edm::ClonePolicy<CSCSegment> >,edm::ClonePolicy<CSCSegment> >",
"edm::RangeMap<DTChamberId,edm::OwnVector<DTRecSegment4D,edm::ClonePolicy<DTRecSegment4D> >,edm::ClonePolicy<DTRecSegment4D> >",
"edmNew::DetSetVector<SiPixelCluster>",
"edmNew::DetSetVector<SiStripCluster>",
"edm::OwnVector<TrackingRecHit,edm::ClonePolicy<TrackingRecHit> >",
"vector<pat::CompositeCandidate> >",
"vector<pat::Electron> >",
"vector<pat::Jet> >",
"vector<pat::MET> >",
"vector<pat::Muon> >",
"vector<pat::Photon> >",
"vector<pat::Tau> >",
"vector<pat::TriggerObjectStandAlone> >",
"edm::OwnVector<reco::BaseTagInfo,edm::ClonePolicy<reco::BaseTagInfo> > >",
"vector<reco::Conversion> >",
"vector<reco::TrackExtra> >",
"vector<reco::Vertex> >",
]
for p in badproducts_mini:
findbad(p) It is definitely not working fully correctly, as all pat objects should have the |
So for the miniaod file Looking at the class |
Scratch what I wrote about |
More info on the use of I say we nuke it from orbit, as it is the only way to be sure. |
I updated the scan script to recurse through subclasses, and other than showing that (as expected) all
which I added above as well |
So I did a 'hack' where I kept the type as The problem is running @nsmith- 's python program that tests RNTuple compatibility. When I run that in my new area, I get the failure
Where @pcanal does RNTuple have problems with enums in classes? |
@pcanal I see another failure
|
So I added entries to classes_def.xml to explicitly force the creating of dictionaries for the enum types, but the build failed with
|
I tried switching So it seems Nick's script which tries to figure out why RNTuple rejects the type says that RNTuple is requiring a dictionary for an enum but genreflex refuses to generate one. |
I would guess that there is an issue with how my script works, in that if a class cannot be made into an RNTuple field, and it contains items that RNTuple would have skipped over, it would claim those are the problem even when they are (probably) not actually serialized. I don't know if I'm duplicating the logic correctly, nor did I really want to duplicate it! What happens if you just try model = ROOT.Experimental.RNTupleModel.Create()
model.MakeField["CSCRecHit2D"]("test") ? |
|
From that printout I noticed you're using ROOT 6.26. Did you mean to start from |
I noticed the same thing myself from the printout. I've already moved my changes to _ROOT630 version and am rebuilding as I type. |
So switching to ROOT 6.30 I get different problems. The DTRecSegment4D seems to now go farther and then hits a import ROOT
model = ROOT.Experimental.RNTupleModel.Create()
model.MakeField["CSCSegment"]("test") just doing
doesn't have a problem. |
So the seg fault seems to be from an infinite recursion happening
|
I think I found it, a |
Oh, we can tick off the recursion box too. This could be worth of CMSSW issue of its own (as the issue is different from |
In
CMSSW_14_0_0_pre0_ROOT630
the following MiniAOD (SIM) branches are not interpretable by RNTupleMuon hits
From cms-sw#42734 (comment) the
edm::OwnVector
can be replaced withstd::vector
edm::OwnVector<CSCSegment>
to astd::vector
edm::OwnVector<DTRecSegment4D>
to astd::vector
edm::RangeMap
template can be migrated to persist usingstd::vector
[RFC] Replace RangeMap used in Muon hit storage cms-sw/cmssw#42917Muon inner track data
These are already supported in ROOT master.
From cms-sw#42734 (comment) the class is using polymorphism
edm::OwnVector<TrackingRecHit>
data productsThis is labeled as
slimmedMuonTrackExtras
, so cms-sw#42734 (comment) may be relevant for testing the muon tracks can be re-fitted from MiniAOD written after the migration.oniaPhotonCandidates
This is the collection
patCompositeCandidates_oniaPhotonCandidates_conversions_PAT
.pat::TriggerObjectStandAlone
RefToBase to Ptr conversion for pat::TriggerObjectStandAlone #5edm::OwnVector<pat::UserData> userDataObjects_
is unused and removeElectrons
ULong64_t
Switch unsigned long long for uint64_t in reco::PFCandidate #3edm::OwnVector<reco::Candidate> dau
PFCandidate no longer inherits from CompositeCandidate #8isoDeposits_
Replaced std::multimap in reco::IsoDeposit #2reco::GsfElectron conversionRejection_
Jets
ULong64_t
Switch unsigned long long for uint64_t in reco::PFCandidate #3edm::OwnVector<reco::Candidate> dau
PFCandidate no longer inherits from CompositeCandidate #8edm::OwnVector<reco::BaseTagInfo>
Removed embedded BaseTagInfo in pat::Jet #9reco::JPTJet::Specific
pat::TriggerObjectStandAlone
RefToBase to Ptr conversion for pat::TriggerObjectStandAlone #5edm::OwnVector<pat::UserData> userDataObjects_
is unused and removeMET
pat::TriggerObjectStandAlone
RefToBase to Ptr conversion for pat::TriggerObjectStandAlone #5edm::OwnVector<pat::UserData> userDataObjects_
is unused and removeMuons
ULong64_t
Switch unsigned long long for uint64_t in reco::PFCandidate #3edm::OwnVector<reco::Candidate> dau
PFCandidate no longer inherits from CompositeCandidate #8isoDeposits_
Replaced std::multimap in reco::IsoDeposit #2reco::Muon refittedTrackMap_
Photons
isoDeposits_
Replaced std::multimap in reco::IsoDeposit #2pat::TriggerObjectStandAlone
RefToBase to Ptr conversion for pat::TriggerObjectStandAlone #5edm::OwnVector<pat::UserData> userDataObjects_
is unused and removeTaus
ULong64_t
Switch unsigned long long for uint64_t in reco::PFCandidate #3edm::OwnVector<reco::Candidate> dau
PFCandidate no longer inherits from CompositeCandidate #8edm::RefToBase<reco::Jet> pfJetRef_
isoDeposits_
Replaced std::multimap in reco::IsoDeposit #2Trigger objects
pat::TriggerObjectStandAlone
RefToBase to Ptr conversion for pat::TriggerObjectStandAlone #5Jet tagInfos
This is
recoBaseTagInfosOwned_slimmedJetsPuppi_tagInfos_PAT
. From cms-sw#42734 (comment) the class is using polymorphism.Conversions
TODO: is the poor-mans constexpr here really being persisted?
vector<edm::RefToBase<reco::Track> > tracks_
More muon track data
TODO: is the poor-mans constexpr here really being persisted?
edm::RefToBase<TrajectorySeed> seedRef_
Vertices
TODO: is the poor-mans constexpr here really being persisted?
vector<edm::RefToBase<reco::Track> > tracks_
@Dr15Jones @makortel may be interested in this
The text was updated successfully, but these errors were encountered: