gaioguys / GAIO.jl

A Julia package for set oriented computations.
MIT License
8 stars 4 forks source link

more verbose automatic testing / benchmarking + progress bars #98

Closed April-Hannah-Lena closed 6 months ago

April-Hannah-Lena commented 9 months ago

I have two options for optional progressmeters that are more elegant + optimized compared to the current idea. For reference, here is the current code in the PR:

Current PR version

function map_boxes(
        g::IntervalBoxMap, source::BS;
        show_progress=false
    ) where {B,Q,S,BS<:BoxSet{B,Q,S}}

    prog = Progress(length(source)+1; desc="Computing image...", enabled=show_progress, showspeed=true)

    P = source.partition
    @floop for box in source
        c, r = box
        int = IntervalBox(box)
        minced = mince(int, g.n_subintervals(c, r))
        show_progress && @reduce( n_ints_mapped = 0 + length(minced) )
        for subint in minced
            fint = g.map(subint)
            fbox = Box(fint)
            isnothing(fbox) && continue
            fSet = cover(P, fbox)
            @reduce( image = BoxSet(P, S()) ⊔ fSet )
        end
        next!(prog)
    end

    next!(prog, showvalues=[("Total number of mapped intervals", n_ints_mapped)])
    return image::BS
end

Now, here is the option where progressmeter and mno progressmeter are written in two separate functions:

Option 1

function map_boxes(
        g::IntervalBoxMap, source::BS,
        show_progress::Val{false}
    ) where {B,Q,S,BS<:BoxSet{B,Q,S}}

    P = source.partition
    @floop for box in source
        c, r = box
        int = IntervalBox(box)
        minced = mince(int, g.n_subintervals(c, r))
        for subint in minced
            fint = g.map(subint)
            fbox = Box(fint)
            isnothing(fbox) && continue
            fSet = cover(P, fbox)
            @reduce( image = BoxSet(P, S()) ⊔ fSet )
        end
    end
    return image::BS
end

function map_boxes(
        g::IntervalBoxMap, source::BS,
        show_progress::Val{true}
    ) where {B,Q,S,BS<:BoxSet{B,Q,S}}

    prog = Progress(length(source)+1; desc="Computing image...", showspeed=true)
    P = source.partition
    @floop for box in source
        c, r = box
        int = IntervalBox(box)
        minced = mince(int, g.n_subintervals(c, r))
        @reduce( n_ints_mapped = 0 + length(minced) )
        for subint in minced
            fint = g.map(subint)
            fbox = Box(fint)
            isnothing(fbox) && continue
            fSet = cover(P, fbox)
            @reduce( image = BoxSet(P, S()) ⊔ fSet )
        end
        next!(prog)
    end

    next!(prog, showvalues=[("Total number of mapped intervals", n_ints_mapped)])
    return image::BS
end

This is much simpler to read, but is twice as long. Another version is to use some metaprogramming:

Option 2

initialize = :( prog = Progress(length(source)+1; desc="Computing image...", showspeed=true) )
count_points = :( @reduce( n_ints_mapped = 0 + length(minced) ) )
update = :( next!(prog) )
finalize = :( next!(prog, showvalues=[("Total number of mapped intervals", n_ints_mapped)]) )

for show_progress in [true, false]

    # how to read: for show_progress == false, imagine deleting all lines with $(...)
    @eval function map_boxes(
            g::IntervalBoxMap, source::BS,
            show_progress::Val{$show_progress}
        ) where {B,Q,S,BS<:BoxSet{B,Q,S}}

        $(show_progress ? initizalize : :())
        P = source.partition
        @floop for box in source
            c, r = box
            int = IntervalBox(box)
            minced = mince(int, g.n_subintervals(c, r))
            $(show_progress ? count_points : :())
            for subint in minced
                fint = g.map(subint)
                fbox = Box(fint)
                isnothing(fbox) && continue
                fSet = cover(P, fbox)
                @reduce( image = BoxSet(P, S()) ⊔ fSet )
            end
            $(show_progress ? update : :())
        end

        $(show_progress ? finalize : :())
        return image::BS
    end

end

This is more optimized than the original, and I think the most elegant, though it is maybe not much easier to read than the original PR option.

gaioguy commented 9 months ago

I actually prefer option 1. Plus some comment above the second function that this is functionally identical to the first one.

gaioguy commented 7 months ago

There is still some check in the documentation that does not pass ...

April-Hannah-Lena commented 7 months ago

Sorry, that was a miscommunication on my part. The next PR has bugfixes, then a docs update. I pushed a smaller change to restrict the version of Documenter.jl.