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

decide if [1 2 3] should be a matrix or row vector #23018

Closed
StefanKarpinski opened this issue Jul 29, 2017 · 45 comments
Closed

decide if [1 2 3] should be a matrix or row vector #23018

StefanKarpinski opened this issue Jul 29, 2017 · 45 comments
Assignees
Labels
linear algebra Linear algebra
Milestone

Comments

@StefanKarpinski
Copy link
Sponsor Member

There was some discussion of this now that we have row vectors.

@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Jul 29, 2017
@JeffBezanson JeffBezanson added needs decision A decision on this change is needed linear algebra Linear algebra labels Jul 29, 2017
@ararslan ararslan changed the title decide of [1 2 3] should be a matrix or row vector decide if [1 2 3] should be a matrix or row vector Jul 29, 2017
@ararslan
Copy link
Member

Would this refer to the space-delimited array literal only or also to hcat on scalars? Seems like the two should be consistent.

@dlfivefifty
Copy link
Contributor

+1 for RowVector

1 similar comment
@DrKrar
Copy link
Contributor

DrKrar commented Jul 29, 2017

+1 for RowVector

@andreasnoack
Copy link
Member

+1 for Matrix because then we have simple ways to construct a Matrix ([1 2 3]) as well as a RowVector ([1,2,3]').

@dlfivefifty
Copy link
Contributor

dlfivefifty commented Jul 30, 2017

Won't [1 2 3;] still create a matrix either way?

@andreasnoack
Copy link
Member

Good question. I don't know. That might be part of the decision here.

@JeffBezanson
Copy link
Sponsor Member

Yes, the parser distinguishes [1 2 3] and [1 2 3;], which could be useful for this case, but which I'm also not a fan of. It means there are three syntax forms ([1,2], [1 2], and [1 2; ...]) for each kind of brackets, which will start to be annoying if we add more brackets. Currently {1 2} and {1 2;} parse the same.

@perrutquist
Copy link
Contributor

It would be an interesting experiment to implement this and see how much code it breaks in published packages. My guess is that most code would keep running, and it will turn out to be very rare that people actually wanted a 1-by-n matrix rather than a row vector when they wrote [1 2 3].

(Personally, I don't think I'll ever want to create a m-by-1 or 1-by-n matrix from scalars, but If I ever do, I'll be happy to use a more verbose syntax, such as Matrix([1 2 3]).)

@JeffBezanson
Copy link
Sponsor Member

There's also this:

julia> a, b = [1,2]', [3,4]'

julia> [a b]
1×4 RowVector{Int64,Array{Int64,1}}:
 1  2  3  4

julia> [a b;]
1×4 Array{Int64,2}:
 1  2  3  4

@dlfivefifty
Copy link
Contributor

If that behaviour is maintained, then it makes no sense that [1 2] is a Matrix

@Keno Keno removed the needs decision A decision on this change is needed label Aug 3, 2017
@StefanKarpinski
Copy link
Sponsor Member Author

Triage decision is not to promote to matrix for scalar hcat (what @dlfivefifty said :).

@andreasnoack
Copy link
Member

andreasnoack commented Aug 4, 2017

The thing I was trying to say during the call was that we might get some feedback when

["A" "B" "C"]

starts returning

julia> RowVector(["A", "B", "C"])
1×3 RowVector{Any,Array{String,1}}:
Error showing value of type RowVector{Any,Array{String,1}}:
ERROR: MethodError: no method matching transpose(::String)
Closest candidates are:
  transpose(::BitArray{2}) at linalg/bitarray.jl:265
  transpose(::Number) at number.jl:100
  transpose(::RowVector{T,CV} where CV<:(ConjArray{T,1,V} where V<:(AbstractArray{T,1} where T) where T) where T) at linalg/rowvector.jl:82
  ...
Stacktrace:
 [1] _getindex at ./abstractarray.jl:906 [inlined]
 [2] getindex(::RowVector{Any,Array{String,1}}, ::Int64, ::Int64) at ./abstractarray.jl:882
 [3] isassigned(::RowVector{Any,Array{String,1}}, ::Int64, ::Int64, ::Vararg{Int64,N} where N) at ./abstractarray.jl:222
 [4] alignment(::IOContext{Base.Terminals.TTYTerminal}, ::RowVector{Any,Array{String,1}}, ::Array{Int64,1}, ::Array{Int64,1}, ::Int64, ::Int64, ::Int64) at ./show.jl:1357
 [5] print_matrix(::IOContext{Base.Terminals.TTYTerminal}, ::RowVector{Any,Array{String,1}}, ::String, ::String, ::String, ::String, ::String, ::String, ::Int64, ::Int64) at ./show.jl:1487
 [6] print_matrix(::IOContext{Base.Terminals.TTYTerminal}, ::RowVector{Any,Array{String,1}}, ::String, ::String, ::String) at ./show.jl:1459
 [7] #showarray#263(::Bool, ::Function, ::IOContext{Base.Terminals.TTYTerminal}, ::RowVector{Any,Array{String,1}}, ::Bool) at ./show.jl:1709
 [8] display(::Base.REPL.REPLDisplay{Base.REPL.LineEditREPL}, ::MIME{Symbol("text/plain")}, ::RowVector{Any,Array{String,1}}) at ./REPL.jl:122
 [9] display(::Base.REPL.REPLDisplay{Base.REPL.LineEditREPL}, ::RowVector{Any,Array{String,1}}) at ./REPL.jl:125
 [10] display(::RowVector{Any,Array{String,1}}) at ./multimedia.jl:194
 [11] eval(::Module, ::Any) at ./boot.jl:235
 [12] print_response(::Base.Terminals.TTYTerminal, ::Any, ::Void, ::Bool, ::Bool, ::Void) at ./REPL.jl:144
 [13] print_response(::Base.REPL.LineEditREPL, ::Any, ::Void, ::Bool, ::Bool) at ./REPL.jl:129
 [14] (::Base.REPL.#do_respond#16{Bool,Base.REPL.##26#36{Base.REPL.LineEditREPL,Base.REPL.REPLHistoryProvider},Base.REPL.LineEditREPL,Base.LineEdit.Prompt})(::Base.LineEdit.MIState, ::Base.AbstractIOBuffer{Array{UInt8,1}}, ::Bool) at ./REPL.jl:646

i.e. a something you cannot index into.

Possible solutions seem to be

  1. Tell people to use ["A" "B" "C";]
  2. Return a Matrix (which we decided against here)
  3. Return a yet to be implemented lazy vector transpose type for non-linear algebra use

@dlfivefifty
Copy link
Contributor

dlfivefifty commented Aug 4, 2017

There's talk of making transpose non-recursive, which resolves this: #20978

See also https://discourse.julialang.org/t/proposal-rename-rowvector-to-transposevector-add-rowvector-that-doesnt-transposes-entries/4681/3

Obviously, when A and B are matrices, with the current RowVector, [A B] should be RowVector([A',B']).

Edit: that was a silly example since it should be a Matrix

@andreasnoack
Copy link
Member

I'm very much aware of #20978 and I'm not too confident that it easily fixed. Instead 3. above, we could change the behavior of RowVector to be non-recursive but then we'd need a new type that is doing what RowVector is doing now so that is just a different version of 3.

@StefanKarpinski
Copy link
Sponsor Member Author

StefanKarpinski commented Aug 4, 2017

We can record here that modulo type issues due to #20978, there seems to be preference for [1 2 3] being a row vector. It doesn't seem to be crucial however, if it turns out to be impractical.

@andyferris
Copy link
Member

OK, so between recursive adjoints, non-recursive transposes, the need to support all the fast BLAS stuff and lazy conjugation, the need to be able to extract a row of a Matrix as a RowVector, and this, it seems that we need to RowVectors which have different views on their elements - i.e. a RowVector which maps it's elements (lazily) through identity, conj, adjoint and x -> conj(adjoint(x)).

While I was really hoping to avoid recursive adjoints altogether, I guess we just need to do this. Similarly for the upcoming lazy matrix adjoint/transpose.

@andyferris
Copy link
Member

Two design questions:

  • Is it OK to introduce a conjadjoint function?
  • Would we like to introduce a lazy MappedArray to do the element mapping? Or have the RowVector store this?

@andreasnoack
Copy link
Member

Would you then apply different functions depending on the element type?

@StefanKarpinski
Copy link
Sponsor Member Author

Would it make sense to have RowVector be parameterized by a transform function? The only one above that doesn't have a standard name (and identity) is cadjoint.

@andyferris
Copy link
Member

Stefan - yes exactly, we can either do that or extend the ConjArray idea. At some point someone will do (vec') .' and expect laziness so IMO in either case we still need ConjArray (plus AdjointArray and ConjAdjointArray) in Base (or else a general MappedArray).

Andreas - I'm guessing that these will all just get inlined away for e.g. Real elements so that wouldn't be strictly necessary. But we could automatically simplify these at construction time so that adjoint(::AbstractVector{<:Real}) doesn't worry about recursive adjoint or any conjugation and just chooses an identity function or whatever. But in the general case all the combinations must be tracked.

@andreasnoack
Copy link
Member

Wouldn't it mean that we'd have to define hcat for every new type that would like to opt out of the recursive behavior, i.e. to use identity instead of adjoint? If so, we'd just have moved the problem to a different place.

@dlfivefifty
Copy link
Contributor

I thought the idea is RowVector no longer transposes the entries, so hcat implementation is completely independent of adjoint she.

@andreasnoack
Copy link
Member

I was referring to #23018 (comment) where it was suggested that RowVector should include a function which is applied to the elements on access.

@andyferris
Copy link
Member

For hcat that function would be identity.

Though I'm thinking that extending ConjArray to AdjointArray etc to manage recursion and have no such transformation would be cleanest.

@StefanKarpinski StefanKarpinski added the stdlib Julia's standard library label Nov 20, 2017
@StefanKarpinski
Copy link
Sponsor Member Author

This is a linear-algebra operation so could be decoupled from Base, but then the question is if we require doing using LinAlg before you can do [1 2 3] and get a row vector. Seems harsh.

@StefanKarpinski StefanKarpinski added triage This should be discussed on a triage call and removed stdlib Julia's standard library labels Nov 20, 2017
@StefanKarpinski
Copy link
Sponsor Member Author

Triage feels that this should be a row vector.

@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Nov 20, 2017
@andyferris
Copy link
Member

The plan in #23424: transpose is not a linear algebra operation (though LinAlg understands it) and RowVector is in Base not LinAlg, so this change seems consistent to me. From memory it was pretty easy to implement.

The only thing I'd really like to consider is that [1 2 3;] be a matrix... Thoughts?

@StefanKarpinski
Copy link
Sponsor Member Author

I'd be happy with that – anything with both spaces and semicolons as a matrix.

@JeffBezanson
Copy link
Sponsor Member

I'm ok with that too, but it's a pity there's no corresponding way to get an Nx1 object from [1, 2] (is there?). I guess that's what you get for using space as a delimiter.

@Sacha0
Copy link
Member

Sacha0 commented Nov 21, 2017

it's a pity there's no corresponding way to get an Nx1 object from [1, 2] (is there?)

Close enough perhaps?

julia> [1; 2]
2-element Array{Int64,1}:
 1
 2

@JeffBezanson
Copy link
Sponsor Member

[1; 2] and [1, 2] are both 1-d; I mean adding something to that syntax to get a 2-d, Nx1 object.

@Sacha0
Copy link
Member

Sacha0 commented Nov 21, 2017

Ah, sorry! My expectation that [1; 2] yields a Matrix was so strong that I did not notice it clearly returning a Vector above 😄.

@andyferris
Copy link
Member

To be honest I've always been confused why [1; 2] isn't a 2x1 matrix. We'd have to change lowering of this from vcat to something else.

@JeffBezanson
Copy link
Sponsor Member

I would think [a; b] should concatenate two vectors, giving another vector.

@andyferris
Copy link
Member

andyferris commented Nov 22, 2017

I guess I've always found this syntax meaning to be a little unnecessary when we have vcat(a, b) which is clear and easily typed, and we otherwise use semicolons for our matrix literals.

I also find that my brain assumes [...] makes an Array but the result of vcat can be anything (same for hcat and hvcat, of course).

@mbauman
Copy link
Sponsor Member

mbauman commented Jan 5, 2018

I must confess that this just feels slightly less obvious to me now that we no longer have a specific RowVector and instead use the more general Adjoint and Transpose wrappers. It's no longer clear to me that the proposed behavior:

julia> [1 2 3]
1×3 Transpose{Int64,Array{Int64,1}}:
 1  2  3

is any better than the status quo. I know it's semantically the same as the old RowVector… it just feels different.

@fredrikekre
Copy link
Member

Also worth considering that the those wrappers likely will move out with LinAlg at some point.

@JeffBezanson
Copy link
Sponsor Member

As much as I like moving things out of Base, I don't think it should drive everything. If we want [1 2 3] to give a row vector, and we want that syntax available by default, then those wrappers might have to stay in Base; fine.

@Sacha0
Copy link
Member

Sacha0 commented Jan 5, 2018

I share Andreas's view on this question. Additionally, the matrix literal / array concatenation syntax leaves something to be desired as it stands, and adding another special case sounds less than fantastic. Best!

@JeffBezanson
Copy link
Sponsor Member

But what about...THIS: #23018 (comment)

@andyferris
Copy link
Member

So with the implementation of recursive transposed row vector, I would still love to see [“a” “b”] not throw an error (it has to make a 1x2 Matrix{String} to avoid an errors).

I mean, we could reserve this syntax for linear algebra usage and require ; as Jeff suggests for the matrix literal, I suppose.

It could potentially get a bit yuck with the storage of the elements of this literal being recursively transposed compared to how they are written - especially for element types that don’t hcat but do transpose.

@Sacha0
Copy link
Member

Sacha0 commented Jan 7, 2018

But what about...THIS: #23018 (comment)

That sort of oddity would be lovely to do away with in sorting out #7128. Best!

@dlfivefifty
Copy link
Contributor

I was under the impression that the proposal was for ["a" "b"] to return RowVector(["a","b"]) and that RowVector is non-recursive (unlike Transpose and Adjoint)?

@JeffBezanson
Copy link
Sponsor Member

That sort of oddity would be lovely to do away with in sorting out #7128. Best!

I don't think that resolves this. Solving #7128 in general appears to be harder; indeed there is no solution in the pipeline. Currently, [a b] unambiguously means concatenation. If we changed the syntax for concatenation, the new syntax would have the same issue. Or, if we added syntax for non-concatenating matrix construction, say [| |], would [|a b|] give a matrix or a row vector?

@JeffBezanson JeffBezanson added the triage This should be discussed on a triage call label Jan 16, 2018
@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Jan 18, 2018
@StefanKarpinski
Copy link
Sponsor Member Author

Triage feels that having [1 2 3] produce a plain ol' matrix is simplest and least surprising.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra
Projects
None yet
Development

No branches or pull requests