Promote_shape dimension mismatch

General Usage
1

Hey,

I saw an error message:
ERROR: LoadError: DimensionMismatch("dimensions must match: a has dims (Base.OneTo(2), Base.OneTo(3), Base.OneTo(2), Base.OneTo(6)), b has dims (Base.OneTo(2), Base.OneTo(3), Base.OneTo(2), Base.OneTo(1)), mismatch at 4")
and I wanted to know what is the use of Base.OneTo in everywhere in the shape checking. Why don’t we just user size(matrix)?

2

Base.OneTo(n) is basically 1:n but the compiler knows for certain (via the type) that the start index is 1.

2 Likes
3

Yes, but why do we need anything like that?

I mean why don’t we just use size(…)?

We increase complexity and ad some redundancy with that without any benefit with the OneTo?

4

The benefit is that

and can thus generate better code in some cases.

1 Like
5

For me this sounds strange, but I understand you.

I could imagine that, size(…) return with a list of Int instead of list of OneTo()-s (axes(…))…? :smiley: The compiler should be able to optimize this too I think. Strange…
For example here: https://github.com/JuliaLang/julia/blob/69fcb5745bda8a5588c089f7b65831787cffc366/base/indices.jl#L169

6

What I think you’re missing is OffsetArrays.jl: axes(A) is allowed to have ranges not starting at 1, so in general has more information than size(A).

3 Likes
7

Damn those stupid OffsetArrays… that unnecessary redundancy that came with it… :smiley: Should be separated pff… :smiley:

8

Damn, sorry to continue but… why don’t we just create another for OffestArrays:

function promote_shape(a::AbstractArray, b::AbstractArray)
    promote_shape(size(a), size(b))
end

function promote_shape(a::OffsetArray, b::OffsetArray)
    promote_shape(axes(a), axes(b))
end

So we could essentially simplify the whole expressions in the end. (Also this is probably one of the most widely used feature, Array aritmetics.)

Also the developer won’t face with a long OneTo(…), OneTo(…)… things when it is just simple a size check in the error messages. :smiley:

9

What you are missing above is that one is free to define types other than OffsetArray with similar functionality, so the first promote_shape(a::AbstractArray, b::AbstractArray) method would give bogus results.

Base.OneTo is just a minor optimization that is mostly a red herring here, the important point is that generalized indexing requires one uses axes etc.

1 Like
10

Yeah, indeed. Then I would just do an otimised version for the Array:

function promote_shape(a::Array{A}, b::Array{B}) where {A,B}
    promote_shape(size(a), size(b))
end

I would do this because OneTo(…) add unnecessary complexity for the error promote for the “Array” types… and I am pretty sure it must add one more little by the OneTo struct in the optimisation process of the compiler… not to mention that I guess 90-99% of the people use the Array/Vector/Matrix most of their time, so it worth the optimisation. (Also julia is all about simplicity and if the core gets simpler it always worths the trade.) What do you think?

11

Pretty much the whole point of Julia is that you get these automatically — a specific version of the method is compiled for each call signature.

I guess one could make this error message prettier (eg print ranges of indices). But that’s orthogonal to the optimization issue.

1 Like
12

I don’t see the why we don’t want to use the optimised version. I understand each point what you are saying. I just proposed an easy optimisation, that could provide better LLVM code faster, which is used crazy amount of times.

We can make specialisation based on our knowledge about the object. Using general version of the method is waste in this scenario.

I know julia is able to do many optimisation and compile the code only once, but I add 1 single specialisation that would make the code easier, cleaner and even faster a little bit at first run.

13

Did you actually check this? This may be an unwarranted assumption.

1 Like
14

No, I just checked broadcast.jl operation.jl and arraymath.jl code and realised this possible improvement option. It would be nice if someone who work on julialang could test it.

15

I tested it in my local environment. I am on 1.6.0.rc1.

using Base: promote_shape

function Base.promote_shape(a::Array{A}, b::Array{B}) where {A,B}
   promote_shape(size(a), size(b))
end

q=randn(3,4)
p=randn(3,2)
p+q

For me it is really clean.
The stacktrace changed from:
ERROR: LoadError: DimensionMismatch("dimensions must match: a has dims (Base.OneTo(3), Base.OneTo(2)), b has dims (Base.OneTo(3), Base.OneTo(4)), mismatch at 2")
to this:
ERROR: LoadError: DimensionMismatch("dimensions must match: a has dims (3, 2), b has dims (3, 4)")
I checked some @btime and @time which shows a very slight improvment on the compilation part (10%) and runtime (0-4%) as we could expect it.

Compilation @time info:
from: (8.3% gc time can be substracted)
# 0.118740 seconds (382.40 k allocations: 21.776 MiB, 8.31% gc time, 99.89% compilation time)
to:
# 0.093647 seconds (354.44 k allocations: 19.927 MiB, 99.89% compilation time)
@btime info:
from: # 7.093 μs (2 allocations: 93.83 KiB)
to: # 6.975 μs (2 allocations: 93.83 KiB)

Do you think I should open a pull request to or an issue to ellaborate on it?

16

Ok, I guess I just post it and the masters will decide anyway…
I really think this could be help and beneficial for newcomers and advanced julianners.

17

It is unclear what and how you are benchmarking here.

Base.promote_shape should be on the order on a few tens of nanoseconds.

Also, note that Base.OneTo also just encodes and integer, so there is no reason to expect a performance improvement.

18

I am talking about compilation time.

Runtime should be optimal, I guess it should be the same.