Skip to content
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

Breaking change in vcat between 0.6.2 and 0.6.3 #27652

Closed
a-parton opened this issue Jun 19, 2018 · 4 comments
Closed

Breaking change in vcat between 0.6.2 and 0.6.3 #27652

a-parton opened this issue Jun 19, 2018 · 4 comments
Labels
arrays [a, r, r, a, y, s]
Milestone

Comments

@a-parton
Copy link

a-parton commented Jun 19, 2018

Between 0.6.2 and 0.6.3 an error appears to have been introduced when concatenating an array with an empty array. I've looked through the commits and the only change that appears to affect vcat is an improvement to the support of abstract arrays, which makes me think that this might not have been a deliberate move.

In 0.6.3:

julia> versioninfo()
Julia Version 0.6.3
Commit d55cadc350 (2018-05-28 20:20 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Haswell)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.9.1 (ORCJIT, broadwell)

julia> vcat([],[1.0 2.0 3.0])
ERROR: ArgumentError: number of columns of each array must match (got (1, 3))
Stacktrace:
 [1] typed_vcat(::Type{Any}, ::Array{Any,1}, ::Array{Float64,2}) at ./abstractarray.jl:1141
 [2] vcat(::Array{Any,1}, ::Array{Float64,2}) at ./sparse/sparsevector.jl:990

and in 0.6.2:

julia> versioninfo()
Julia Version 0.6.2
Commit d386e40c17 (2017-12-13 18:08 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin14.5.0)
  CPU: Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Sandybridge)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.9.1 (ORCJIT, ivybridge)

julia> vcat([],[1.0 2.0 3.0])
1×3 Array{Any,2}:
 1.0  2.0  3.0
@mbauman
Copy link
Member

mbauman commented Jun 19, 2018

Thanks for the report! This is also the case on 0.7, and I think this was indeed an unintended break in behavior for the backport. A quick bisect confirms that 72c3e62 was indeed the culprit, intending to just fix vcat(1:8, fill(1, (4, 1))).

Empty arrays still work, but they must be 0x3:

julia> vcat(reshape([],0,3),[1.0 2.0 3.0])
1×3 Array{Any,2}:
 1.0  2.0  3.0

Should we figure out a way to restore the old behavior on 0.6.4? We're also a little inconsistent in applying this rule:

julia> vcat(zeros(0), rand(2,2,2))
2×2×2 Array{Float64,3}:
#

julia> vcat(zeros(0,2), rand(2,2,2))
ERROR: DimensionMismatch("tried to assign 0×2 array to 0×2×2 destination")
Stacktrace:
 [1] throw_setindex_mismatch at ./indices.jl:138 [inlined]
 [2] setindex_shape_check(::Array{Float64,2}, ::Int64, ::Int64, ::Int64) at ./indices.jl:164
 [3] macro expansion at ./multidimensional.jl:638 [inlined]
 [4] _unsafe_setindex!(::IndexLinear, ::Array{Float64,3}, ::Array{Float64,2}, ::UnitRange{Int64}, ::UnitRange{Int64}, ::UnitRange{Int64}) at ./multidimensional.jl:631
 [5] _setindex! at ./multidimensional.jl:625 [inlined]
 [6] setindex! at ./abstractarray.jl:997 [inlined]
 [7] __cat(::Array{Float64,3}, ::Tuple{Int64,Int64,Int64}, ::Tuple{Bool}, ::Array{Float64,2}, ::Vararg{Any,N} where N) at ./abstractarray.jl:1363

@mbauman mbauman added this to the 0.6.x milestone Jun 19, 2018
@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jun 19, 2018

Should we figure out a way to restore the old behavior on 0.6.4?

Yes — 0.6.4 should endeavor to match the behavior of 0.6.0 except for bugs. If 0.6.3 deviated from that in some way, 0.6.4 should revert the deviation.

ararslan added a commit that referenced this issue Jun 20, 2018
This reverts commit 3a27e01.
As noted in #27652, this change is breaking.
@ararslan
Copy link
Member

I've pushed a reversion for that change in #27621.

@JeffBezanson JeffBezanson added the arrays [a, r, r, a, y, s] label Aug 2, 2018
@JeffBezanson
Copy link
Member

This has been fixed in a 0.6 point release. In 1.0 we give the error, but it is arguably correct. In other cases I believe we'd need to give more errors, which would be a breaking change. That can be discussed separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s]
Projects
None yet
Development

No branches or pull requests

5 participants