Promote hcat vcat #158
Promote hcat vcat #158
Conversation
|
Test failure was on Julia 0.7, I'm tempted to ignore it. |
| @inline Size(::Union{RowVector{T, SA}, Type{RowVector{T, SA}}}) where {T, SA <: StaticArray} = Size(1, Size(SA)[1]) | ||
| @inline Size(::Union{RowVector{T, CA}, Type{RowVector{T, CA}}} where CA <: ConjVector{<:Any, SA}) where {T, SA <: StaticArray} = Size(1, Size(SA)[1]) |
There was a problem hiding this comment.
Oops. This line of code was meant for #157. I suggest we review all changes here and just merge both (or this).
|
failure looks to be this: JuliaLang/julia#21684 (comment) |
src/matrix_multiply.jl
Outdated
| @@ -36,11 +36,13 @@ promote_matprod{T1,T2}(::Type{T1}, ::Type{T2}) = typeof(zero(T1)*zero(T2) + zero | |||
| @inline *(A::StaticMatrix, B::StaticMatrix) = _A_mul_B(Size(A), Size(B), A, B) | |||
| @inline *(A::StaticVector, B::StaticMatrix) = *(reshape(A, Size(Size(A)[1], 1)), B) | |||
| @inline *(A::StaticVector, B::RowVector{<:Any, <:StaticVector}) = _A_mul_B(Size(A), Size(B), A, B) | |||
| @inline *(A::StaticVector, B::RowVector{<:Any, CV} where CV <: ConjVector{<:Any, <:StaticVector}) = _A_mul_B(Size(A), Size(B), A, B) | |||
There was a problem hiding this comment.
Fancy. This can also be written without the where, with B::RowVector{<:Any,<:ConjVector{<:Any,<:StaticVector}}
test/matrix_multiply.jl
Outdated
| @test v2 * v2' === zeros(SMatrix{3, 3, Int}) | ||
|
|
||
| v3 = zeros(SVector{3, Complex{Int}}) | ||
| @test v3 * v3' === zeros(SMatrix{3, 3, Complex{Int}}) |
There was a problem hiding this comment.
Worth testing this a little more specifically, I think. With zeros, the code could do all sorts of things and these tests will still pass.
There was a problem hiding this comment.
I can - this was meant to be a dispatch test (keeping in mind that the matrix-matrix code that this relies upon is checked thoroughly elsewhere)
There was a problem hiding this comment.
I was wondering whether the _A_mul_B(::Size{sa}, ::Size{sb}, a::StaticVector{<: Any, Ta}, b::RowVector{Tb, CV} where CV <: ConjVector{<:Any, <:StaticVector}) was specifically tested?
There was a problem hiding this comment.
Well, it was going to a base method before and now it isn't...
Should tests test for end behaviours or specific things like which method gets invoked?
There was a problem hiding this comment.
In this case, I'd test the end behaviour - the tests will be less brittle that way, and the call chain from public interface to private implementation is not complicated to follow.
In general it's a tradeoff:
- Testing the implementation details can let you write more specific tests for smaller blocks of code. When things break, isolating the problem is easier.
- Testing the interface lets you refactor the implementation without breaking the tests. Refactoring very detailed implementation tests can be a lot of work, and isn't much fun.
There was a problem hiding this comment.
So, after all that - is this version of _A_mul_B tested elsewhere? If not, we should test this (but via the public interface) to make sure we have coverage for that code path.
| @inline Size(::Union{RowVector{T, SA}, Type{RowVector{T, SA}}}) where {T, SA <: StaticArray} = Size(1, Size(SA)[1]) | ||
| @inline Size(::Union{RowVector{T, CA}, Type{RowVector{T, CA}}} where CA <: ConjVector{<:Any, SA}) where {T, SA <: StaticArray} = Size(1, Size(SA)[1]) |
* Set up Documenter * more docs, add action * more docs * fix Documenter script * typo * add to LOAD_PATH * move doctests to runtests.jl * interpolate types into doctest strings
Fixes #155. (Based on #157).