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

Unable to read a LorentzVector from a Tree #880

Closed
bburghgr opened this issue Dec 14, 2021 · 7 comments · Fixed by #881
Closed

Unable to read a LorentzVector from a Tree #880

bburghgr opened this issue Dec 14, 2021 · 7 comments · Fixed by #881

Comments

@bburghgr
Copy link

I'm having difficulty reading from a branch of a tree that contains a TLorentzVector. I first noticed this when writing a short test script for some analysis ntuples, where I was using rtree.Reader to read into an instance of rphys.LorentzVector, but the same error occurs when running groot/cmd/root-dump over a simpler test file.

The test file was created with:

#!/usr/bin/env python2.7

import ROOT
ROOT.gROOT.SetBatch(True)

outF = ROOT.TFile.Open("tree.root", "RECREATE")
outTree = ROOT.TTree("tree", "Test TTree")

p4 = ROOT.TLorentzVector()

# Create branches
outTree.Branch("p4", p4)

for entry in xrange(10):
  p4.SetPxPyPzE(0+entry, 1+entry, 2+entry, 3+entry)
  outTree.Fill()

outF.Write()
outF.Close()
print "Finished"

And the error is:

$ root-dump tree.root 
>>> file[tree.root]
key[000]: tree;1 "Test TTree" (TTree)
panic: reflect.Value.Interface: cannot return value obtained from unexported field or method

goroutine 1 [running]:
reflect.valueInterface({0xb62280, 0xc00002af90, 0x28}, 0x28)
	/usr/local/go/src/reflect/value.go:1362 +0xd9
reflect.Value.Interface(...)
	/usr/local/go/src/reflect/value.go:1351
go-hep.org/x/hep/groot/rdict.(*streamerConfig).adjust(0xc0001fefc0, {0xb5a440, 0xc00002af90})
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rdict/descr.go:52 +0x136
go-hep.org/x/hep/groot/rdict.rstreamTObject(0xc00019f928, {0xb5a440, 0xc00002af90}, 0xc0001ff3e0)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rdict/rstreamer.go:766 +0x2b
go-hep.org/x/hep/groot/rdict.rstreamer.rstream(...)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rdict/rstreamer.go:118
go-hep.org/x/hep/groot/rdict.(*rstreamerInfo).RStreamROOT(0xc0001aba00, 0xc0000001a0)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rdict/decoder.go:100 +0xa3
go-hep.org/x/hep/groot/rtree.(*rleafElem).readFromBuffer(0xc0000002e8, 0xc0001b5ea8)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rtree/rleaf.go:238 +0x22
go-hep.org/x/hep/groot/rtree.(*rbasket).loadRLeaf(0xc000204820, 0x0, {0xda04c8, 0xc0001ff290})
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rtree/rbasket.go:34 +0xbc
go-hep.org/x/hep/groot/rtree.(*rbranch).read(0xc0001aba40, 0xc0001aba40)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rtree/rbranch.go:54 +0x1c9
go-hep.org/x/hep/groot/rtree.(*rtree).read(0xc00007bd10, 0x203000)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rtree/reader.go:452 +0x6b
go-hep.org/x/hep/groot/rtree.(*rtree).run(0xc00007bd10, 0x0, 0x0, 0xa, 0xc0001ff320)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rtree/reader.go:435 +0x139
go-hep.org/x/hep/groot/rtree.(*Reader).Read(0xc0001b57a0, 0xc00016e300)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rtree/reader.go:145 +0xc2
go-hep.org/x/hep/groot/rcmd.(*dumpCmd).dumpTree(0xc000020ba0, {0xdaf3e0, 0xc00016e300})
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rcmd/dump.go:128 +0x1b2
go-hep.org/x/hep/groot/rcmd.(*dumpCmd).dumpObj(0xc000020ba0, {0x7f6f41e14998, 0xc00016e300})
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rcmd/dump.go:82 +0xbf
go-hep.org/x/hep/groot/rcmd.(*dumpCmd).dumpDir(0xc000020ba0, {0xdaa178, 0xc0001fca00})
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rcmd/dump.go:64 +0x2c5
go-hep.org/x/hep/groot/rcmd.Dump({0xd96ae0, 0xc0001ab300}, {0x7ffc15dc92a2, 0xd}, 0x1, 0xbd04b0)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/rcmd/dump.go:44 +0x165
main.dump({0xd96ae0, 0xc0001ab300}, {0x7ffc15dc92a2, 0x9}, 0xb)
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/cmd/root-dump/main.go:123 +0xa5
main.main()
	/home/blake/go/pkg/mod/go-hep.org/x/hep@v0.29.2/groot/cmd/root-dump/main.go:113 +0x3b9

In particular, rstreamer.go:766 seems to be trying to access the rbase.Object inside of the rphys.LorentzVector, so it can directly call UnmarshalROOT on the Object, rather than letting LorentzVector.UnmarshalROOT handle that. Since the object field of the LorentzVector is not exported, reflect panics during the Interface() call on descr.go:52, if I'm following things correctly.

It appears that TLorentzVector is registered in rtypes.Factory out-of-the-box (and calling GoType() on the branch of the tree returns "rphys.LorentzVector"), so it seems like things should be in place. I would naively guess that rstreamSI should be following the case where it finds the root class in the factory, but I think it may actually be following the last return statement, where it ultimately loops over rdict.StreamerInfo.roops and tries to decode the fields one at a time. I haven't tried to test that yet, I thought it better to report the problem in case I'm doing something stupid.

@sbinet
Copy link
Member

sbinet commented Dec 14, 2021

thanks for the report.

I'll have a look.

@bburghgr
Copy link
Author

Looking into this a bit more on my own, I've noticed a few things:

  1. If I simply export the fields of rphys.LorentzVector, then root-dump can run over the test file, though the numbers it spits out seem to be wrong.
  2. When setting up leaves, the call to *StreamerInfo.NewRStreamer seems to be building a streamer with 3 elems.
  3. I suspect the 3 elems are the base object, fP, and fE fields from the generated TLorentzVector info in rdict/cxx_root_streamers_gen.go, rather than one for the rphys.LorentzVector, which I guess would explain why it's not trying to use rphys.LorentzVector.UnmarshalROOT at any point.

@sbinet
Copy link
Member

sbinet commented Dec 16, 2021

yep, NewRStreamer could be smarter.
or, rather, the rbytes.Unmarshaler interface should be smarter and handle member-wise/object-wise unmarshaling.

I have a temporary fix:

diff --git a/groot/rdict/decoder.go b/groot/rdict/decoder.go
index 4502e539..7f5474e1 100644
--- a/groot/rdict/decoder.go
+++ b/groot/rdict/decoder.go
@@ -84,6 +84,16 @@ func (rr *rstreamerInfo) Bind(recv interface{}) error {
                return fmt.Errorf("rdict: invalid kind (got=%T, want=pointer)", recv)
        }
        rr.recv = recv
+       if recv, ok := recv.(rbytes.Unmarshaler); ok {
+               // FIXME(sbinet): handle mbr-/obj-wise
+               rr.rops = []rstreamer{{
+                       op: func(r *rbytes.RBuffer, _ interface{}, _ *streamerConfig) error {
+                               return recv.UnmarshalROOT(r)
+                       },
+                       cfg: nil,
+               }}
+               return nil
+       }
        if len(rr.rops) == 1 {
                se := rr.rops[0].cfg.descr.elem
                if se.Name() == "This" ||

that leads to:

$> root-dump /home/binet/tmp/root-data/tlv.root 
>>> file[/home/binet/tmp/root-data/tlv.root]
key[000]: tree;1 "Test TTree" (TTree)
[000][p4]: {{0 50331648} {{0 50331648} 0 1 2} 3}
[001][p4]: {{0 50331648} {{0 50331648} 1 2 3} 4}
[002][p4]: {{0 50331648} {{0 50331648} 2 3 4} 5}
[003][p4]: {{0 50331648} {{0 50331648} 3 4 5} 6}
[004][p4]: {{0 50331648} {{0 50331648} 4 5 6} 7}
[005][p4]: {{0 50331648} {{0 50331648} 5 6 7} 8}
[006][p4]: {{0 50331648} {{0 50331648} 6 7 8} 9}
[007][p4]: {{0 50331648} {{0 50331648} 7 8 9} 10}
[008][p4]: {{0 50331648} {{0 50331648} 8 9 10} 11}
[009][p4]: {{0 50331648} {{0 50331648} 9 10 11} 12}

sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
This CL adds a temporary mitigation of go-hep#880.
It tries to detect whether a given receiver for binding implements rbytes.Unmarshaler and
use it instead of generating rstreamers.

rstreamers may not properly work in the case where fields of a struct aren't exported.

Updates go-hep#880.
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
@sbinet
Copy link
Member

sbinet commented Dec 16, 2021

see: #881
(and let me know here or there whether it works for you.)

sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
This CL adds a temporary mitigation of go-hep#880.
It tries to detect whether a given receiver for binding implements rbytes.Unmarshaler and
use it instead of generating rstreamers.

rstreamers may not properly work in the case where fields of a struct aren't exported.

Updates go-hep#880.
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
This CL adds a temporary mitigation of go-hep#880.
It tries to detect whether a given receiver for binding implements rbytes.Unmarshaler and
use it instead of generating rstreamers.

rstreamers may not properly work in the case where fields of a struct aren't exported.

Updates go-hep#880.
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
This CL adds a temporary mitigation of go-hep#880.
It tries to detect whether a given receiver for binding implements rbytes.Unmarshaler and
use it instead of generating rstreamers.

rstreamers may not properly work in the case where fields of a struct aren't exported.

Updates go-hep#880.
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
This CL adds a temporary mitigation of go-hep#880.
It tries to detect whether a given receiver for binding implements rbytes.Unmarshaler and
use it instead of generating rstreamers.

rstreamers may not properly work in the case where fields of a struct aren't exported.

Updates go-hep#880.
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
This CL adds a temporary mitigation of go-hep#880.
It tries to detect whether a given receiver for binding implements rbytes.Unmarshaler and
use it instead of generating rstreamers.

rstreamers may not properly work in the case where fields of a struct aren't exported.

Updates go-hep#880.
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
This CL adds a temporary mitigation of go-hep#880.
It tries to detect whether a given receiver for binding implements rbytes.Unmarshaler and
use it instead of generating rstreamers.

rstreamers may not properly work in the case where fields of a struct aren't exported.

Updates go-hep#880.
sbinet added a commit to sbinet-hep/hep that referenced this issue Dec 16, 2021
@bburghgr
Copy link
Author

see: #881 (and let me know here or there whether it works for you.)

Testing the PR at bb66911, things seem to be working. In addition to the test case outlined above, I'm also able to root-dump my analysis ntuples, so that's pretty great.

@sbinet
Copy link
Member

sbinet commented Dec 16, 2021

cool :)

@sbinet
Copy link
Member

sbinet commented Dec 16, 2021

I've created #882 to track the more general issue of reading/writing types with private fields (and leveraging the StreamerInfo machinery).

@sbinet sbinet closed this as completed in 07e2951 Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants