-
Notifications
You must be signed in to change notification settings - Fork 114
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use AbstractVector in types. Add a bit of documentation.
The reason for the switch to `AbstractVector` is to allow for generalization to DArray and SharedArray types.
- Loading branch information
Showing
2 changed files
with
23 additions
and
17 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83e144c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change may have a negative performance impact because, while
Vector{T}
is a concrete type,AbstractVector{T}
is not. It may be better to parametrizeGlmResp
by the vector type instead of the element type.83e144c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you parametrize by the vector type, you may also be able to avoid the changes in 15df539.
83e144c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to specify that the vector type should have an eltype that extends FloatingPoint?
Right now things will fall apart as soon as an LAPACK/BLAS operation is attempted on anything other than Float64 or Float32 vectors. However, with the work by @andreasnoackjensen on generalizing linear algebra methods it is conceivable that a user could work with arrays of Float128 or BigFloat. (I don't really want to think about the Complex cases.)
Should I just throw in the towel and define the vector type as V<:AbstractArray{Float64,1} ?
83e144c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can add
typealias FPAbstractVector{T<:FloatingPoint} AbstractVector{T}
and then parametrize byV<:FPAbstractVector
.83e144c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would there be a value in using
StoredVector
as the base class instead ofAbstractVector
?83e144c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the little I've read of the discussion in JuliaLang/julia#987, I think you're right that
StoredArray
is more appropriate, but others might have a better understanding of the differences.