-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix reading TLeafC
#342
fix reading TLeafC
#342
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
==========================================
+ Coverage 84.23% 84.27% +0.03%
==========================================
Files 19 19
Lines 2557 2563 +6
==========================================
+ Hits 2154 2160 +6
Misses 403 403 ☔ View full report in Codecov by Sentry. |
Not sure if the question is towards me or if I understood it right but just in case I'm replying. Normally each row of the LazyTree should correspond to an Gean4 interaction, including the Particle that interacted, under which process etc. As such, I would expect the first row of the Particle column to be reconstructed as "U238", of the Process column "Radiactivation" and so on. |
Can you show what is expected for each column? I think maybe we're have some offsetting issues or string length encoding mixedup |
converting the root file to csv with uproot and reading it with CSV.jl, that's the format I would be expecting
|
ohh I see, it works like this: julia> Int('\x0f')
15
julia> ['R', 'a', 'd', 'i', 'o', 'a', 'c', 't', 'i', 'v', 'a', 't', 'i', 'o', 'n']
15-element Vector{Char}: yeah.... ok so I'm currently splitting the |
Yep, exactly! |
@GSavvidis this should be fixed now: julia> a.Particle
117364-element LazyBranch{String, UnROOT.Nojagg, Vector{String}}:
"U238"
"alpha"
"Th234"
"Th234"
"e-"
"anti_nu_e"
"anti_nu_e"
"Pa234[166.720X]"
"e-"
"Pa234[103.420X]"
"e-"
"Pa234[73.920X]" it's not super efficient but this should at least work. Let us know if performance with these Strings become an issue |
Incredible! Thanks a lot! Will let you know |
Sorry for coming late to the party. @Moelf the structure is similar to |
Yeah, I dug out the readtype() and reuse that function now |
🙂 should we go for a test file? |
the file provided in the comment above is too large (9.8M), still waiting for confirmation that this is fine (usability and performance) |
Ah sorry ok :) |
I'm having some time constraints so I might need a couple of days to setup my code. In the meantime, it will take me 5 mins to get you a file of 1M(Or even smaller) in size for testing if you want. |
no rush -- feel free to make a smaller file (maybe with 10 events is enough) whenever you've tested your actual use case. |
sounds good! Thanks |
@Moelf Hello, apologies for the delay. I prepared a small file for testing here. |
thanks for the smaller test file! so you're saying for your application the current performance is "good enough"? |
yes. I had to loop through files of more 20 GB in size and with multi-threading I needed about ~160s (18 threads). I didn't have any obvious slow-down. |
fix: https://discourse.julialang.org/t/problem-reading-root-branch-containing-strings-with-unroot-jl/115671/5
I think somehow this is still wrong, but not sure how to fix without knowing what the data should be.