Add naive Base julia AbstractArray implementation #171
Conversation
|
I guess that this will have the effect that certain Julia arrays which are strided but are not part of the Strided(Views).jl should perhaps have an |
|
Ok I've changed and finished the base implementations. Had to do some force pushing to fix a divergence :-). Probably the base implementations need a test. I would also prefer not relying on the |
|
Maybe we can just add a trait for this in Strided.jl, define some defaults for reasonable types and allow people to opt in? |
| function tensoradd!(C::AbstractArray, | ||
| A::AbstractArray, pA::Index2Tuple, conjA::Bool, | ||
| α::Number, β::Number) | ||
| α::Number, β::Number, ::BaseCopy) | ||
| argcheck_tensoradd(C, A, pA) | ||
| dimcheck_tensoradd(C, A, pA) | ||
|
|
||
| # can we assume that C is mutable? | ||
| # is there more functionality in base that we can use? |
There was a problem hiding this comment.
I think there is an in-place permutedims!, such that we could use the allocation interface to also hijack into allocating these temporary arrays
There was a problem hiding this comment.
That might be possible indeed. The question is whether it is worth it. This will probably only be used for types which are very different from strided arrays, e.g. sparse arrays.
There was a problem hiding this comment.
I think that's probably a very valid point. It's probably fair to assume that we cannot guarantee optimal performance without extra information about the specific type anyways, and the system does allow to easily implement custom backends if necessary. The base backend should serve mostly as a catch-all implementation that ensures that it works for most types.
| if iszero(β) | ||
| C .= α .* conj.(reshape(view(Ã, :, 1, 1), so)) | ||
| else | ||
| C .= β .* C .+ α .* conj.(reshape(view(Ã, :, 1, 1), so)) | ||
| end | ||
| for i in 2:st | ||
| C .+= α .* conj.(reshape(view(Ã, :, i, i), so)) | ||
| end |
There was a problem hiding this comment.
can we rewrite this with sum?
There was a problem hiding this comment.
I don't know? In a way that does not cause additional allocations? Is there an issue with the current approach?
There was a problem hiding this comment.
I wrote this in reply to the comment about -- # is there more base functionality we can use. I don't think there is any issue with the current approach.
|
Ok, with these tests included, I think this can be merged, unless there are further comments or suggestions. I will include testing the different backends also in the macro-based tests, but this will first require the other backend specifying syntax. |
This is meant to avoid using the
Stridedbased backends for types that do not support this. For now, it only dispatched to Strided forStridedArrays, and uses a naive version based on permutedims and vectorinterface for the otherAbstractArraysubtypes.My preliminary tests do seem to indicate that this works for things like
FillArrays, so at least this should solve the long-outstanding annoyance where Zygote generates these array types when callingsumon an array, which would then break in TensorOperations. (See also #169 )TODO
tensortraceimplementation