A lazy getindex(::LazyBranch, ::UnitRange)

[grasph]

I've been confused in my first experience with LazyTree with its promptness to materialize and trigger full-branch reads.

Is there any motivation for having lazy_branch[n:m] and lazy_tree[n:m] "eager" or is it for implementation simplification?

I would naturally expect them to limit the range (and offset indexing) without breaking the laziness, while the collect function can be used for a materialization.

Philippe.

[Moelf]

see:

• mention getindex() shouldn't return a "view" JuliaLang/julia#47099

basically, getindex(, ::range) and friends should return a newly allocated array. For view, users should use @view. And we made sure @view works correctly (correct and efficient):

UnROOT.jl/test/runtests.jl

Lines 344 to 351 in c34dea5

	@testset "View" begin
	data = LazyTree(joinpath(SAMPLES_DIR, "tree_with_jagged_array.root"), "t1")
	data[1:2]
	@view data[1:2]
	alloc1 = @allocated v = data[3:90]
	alloc2 = @allocated v = @view data[3:90]
	v = @view data[3:80]
	@test alloc2 < alloc1/100

[Moelf]

notice, nesting @view does exactly what you want: it changes offsets referencing to the same parent array:

julia> a = collect(1:10);

julia> b = @view a[1:5];

julia> c = @view b[1:3];

julia> c
3-element view(::Vector{Int64}, 1:3) with eltype Int64:
 1
 2
 3

julia> reverse!(a);

julia> c
3-element view(::Vector{Int64}, 1:3) with eltype Int64:
 10
  9
  8

[grasph]

The situation with Array is different because it has a setindex! method and it's itself an Array.

The proposal would be that getindex on LazyBranch returns a LazyBranch, similarly to getindex on Tuple that returns a Tuple and getindex on Array that returns an Array.

So getindex will allocate a new LazyBranch and the LazyBranch struct will have two extra fields to keep track of the offset and maximum length.

Philippe.

[Moelf]

The proposal would be that getindex on LazyBranch returns a LazyBranch

getindex() of LazyTree over a range does return you LazyTree.

Btw, I think I should clarify that I see LazyBranch as mostly an internal implementation detail subject to change, LazyTree is the only public-facing thing. It's fine to plug LazyBranch into DataFrame columns as part of the Table-container swapping, but I don't think people should use it directly too much

[Moelf]

the guideline on "getindex() shouldn't return lazy view" is for all <:AbstractArray. What's wrong with view btw?

If tree[n:m] is lazy, what differentiate it from @view tree[n:m]? and when user wants to materialize do they call collect() or copy()?

We have argued copy() shouldn't allocate. So maybe collect(); but careful, do you want collect(tree[n:m]) to return something different from [i for i in tree[n:m]]?

If you want to keep the property collect(tree[n:m]) == [evt for evt in tree[n:m]], then you need a third function, say UnROOT.materialize(tree[n:m]). I find this confusing

[grasph]

He will use collect(). The purpose of collect is to return an Array from a collection or an iterable, so it matches exactly the intention.

There is nothing wrong with view, it is just that the getindex behavior is confusing: it is counterintuitive that adding a 1:n to limit the number of processed events in a for e in tree loop will increase the memory usage by loading the n entries in memory.

I don't see contradiction from my proposal with the guidelines: you wrote that it must allocate a copy, it allocates a copy of the LazyBranch which is your "array" (in the sense of subtyping AbstractArray). The LayzyBranch contains references to data stored in a file, "copy" just means copying the reference but not the referred content, as it's the case for the elements of an array that are references. The important point is what was the rationale of the guideline. Maybe there is some reason I missed that required a copy of the data

[Moelf]

If view has not practical issues then I would still think the current behavior is less surprising and more consistent with wider Julia ecosystem:

Arrow.jl

julia> df = Arrow.Table("/tmp/a.feather");

julia> typeof(df.x) # this is memory-mapp, i.e. lazy
Arrow.Primitive{Int64, Vector{Int64}}

julia> typeof(df.x[1:3])
Vector{Int64} (alias for Array{Int64, 1})

MappedArrays

julia> using MappedArrays

julia> a = mappedarray(sqrt, [1,3,4]);

julia> typeof(a) # lazy because `sqrt` only evaluates upon indexing
ReadonlyMappedArray{Float64, 1, Vector{Int64}, typeof(sqrt)}

julia> typeof(a[1:2])
Vector{Float64} (alias for Array{Float64, 1})

---

collect() is not as natural as you may have assumed -- collect() promises to return a Array, not <:AbstractArray, so we cannot use it for the purpose you're suggesting. I will highlight the difference:

julia> t = LazyTree(UnROOT.samplefile("NanoAODv5_sample.root"), "Events", r"Muon_(pt|eta|phi)$");

julia> collect(t[1:2])
2-element Vector{UnROOT.LazyEvent}:
 UnROOT.LazyEvent at index 1 with 3 columns:
(Muon_phi = Float32[], Muon_pt = Float32[], Muon_eta = Float32[])
 UnROOT.LazyEvent at index 2 with 3 columns:
(Muon_phi = Float32[-0.30541992, 0.98999023], Muon_pt = Float32[19.93826, 15.303187], Muon_eta = Float32[0.53015137, 0.2286377])

julia> t[1:2] # materialized but still is a table
 Row │ Muon_phi         Muon_pt          Muon_eta
     │ Vector{Float32}  Vector{Float32}  Vector{Float32}
─────┼───────────────────────────────────────────────────
 1   │ []               []               []
 2   │ [-0.305, 0.99]   [19.9, 15.3]     [0.53, 0.229]

we cannot make collect(t[1:2]) do what current t[1:2] is doing

[grasph]

Let's conclude that it is natural for Julia experts, but not for a particle physicist. I still think that with the size of the file we are dealing with, copy of the file content into memory should be avoided, unless explicitly requested by the user.

[Moelf]

I highly appreciate the intention and we definitely try our best to design everything for the target users whenever possible. But at some point users gotta know what they are doing, we can't make things surprising for everyone except the most naive user who just tried whatever came to their mind.

I just feel like if we tailors designs around "day 1 users", it's not good for the long term. And I believe given HEP physicists literacy in software, it would be good to be consistent with the ecosystem we're in.

I think we have room to improve for sure, and let's keep the discussion open. I am also open to other proposals if any come across your mind in the future.

[Moelf]

for example, I think the first thing scientific Python users coming to Julia find surprising is that ary[n:m] makes a copy. One could argue if they're designing a package targeting ex-Python users, maybe they should alter those semantics and return a view() for some custom types.

But I would argue they should document and leave a note (or point to existing notes from documentation: https://docs.julialang.org/en/v1/manual/performance-tips/#man-performance-views), I don't think it's realistic to expect our users to only use JuliaHEP packages and thus we can break away from ecosystem convention.

[grasph]

An interesting AbstractArray case: an n:m slice of UnitRange is a UnitRange, while slicings that can lead to a set of non-contiguous elements return a Vector.

julia> r = 1:5
1:5

julia> isa(r, AbstractVector)
true

julia> r[2:3]
2:3

julia> r[:]
1:5

julia> r[[1,2,3,4,5]]
5-element Vector{Int64}:
 1
 2
 3
 4
 5

julia> r[iseven.(r)]
2-element Vector{Int64}:
 2
 4

[Moelf]

yeah, I think Jameson's point is that, after copy(), user operation on the copy shouldn't affect the original.