MeasureBase.jl icon indicating copy to clipboard operation
MeasureBase.jl copied to clipboard

Missed simplification opportunity for `productmeasure`

Open cscherrer opened this issue 2 years ago • 8 comments

In this measure

julia> d = productmeasure(fill(Lebesgue(), 5))
ProductMeasure([Lebesgue(ℝ), Lebesgue(ℝ), Lebesgue(ℝ), Lebesgue(ℝ), Lebesgue(ℝ)])

we have

julia> Base.issingletontype(eltype(marginals(d)))
true

But this guarantees we could instead have written it as a power measure. Let's update the powermeasure to account for this.

cscherrer avatar Sep 05 '23 21:09 cscherrer

Oh, nice!

oschulz avatar Sep 06 '23 07:09 oschulz

But you would lose type stability no? Can you infer the type just from the input arguments?

Unless you want to make PowerMeasure a special case of ProductMeasure?

theogf avatar Sep 06 '23 10:09 theogf

I think it will still be type-stable. When we call

productmeasure(::AbstractArray{T,N})

the rule can fire or not depending on issingletontype(T). I think it's enough for it to be a function of the type like this. Am I missing something?

cscherrer avatar Sep 06 '23 13:09 cscherrer

Currently (in #120) we have

_generic_productmeasure_impl(mar::AbstractArray) = ProductMeasure(asmeasure.(mar))

We could change this to

function _generic_productmeasure_impl(mar::AbstractArray{T}) where {T}
    if Base.issingletontype(T)
        first(mar) ^ size(mar)
    else
        ProductMeasure(asmeasure.(mar))
    end
end

cscherrer avatar Sep 06 '23 19:09 cscherrer

I guess we could even make it

if Base.issingletontype(T) || allequal(mar)

This requires walking through mar, and the fallback asmeasure.(mar) will require another traversal. It'd be better if we could fuse those. Shouldn't be too hard...

cscherrer avatar Sep 06 '23 20:09 cscherrer

Keeping it simple for now. Added this as a test:

julia> productmeasure(fill(Lebesgue(), 5)) isa PowerMeasure
true

cscherrer avatar Sep 06 '23 20:09 cscherrer

Maybe add @inferred to the test?

oschulz avatar Sep 07 '23 08:09 oschulz

Good idea! Done.

cscherrer avatar Sep 07 '23 13:09 cscherrer