-
Notifications
You must be signed in to change notification settings - Fork 37
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
Introduce a isa_wrapped_array
function
#460
Comments
Ok think I get it. (and |
Correct. I am still debating if it should be
Broader than that. Any array type that wraps over "primitive" array types (examples being julia> dl = ones(3); d = ones(4); du = ones(3);
julia> X = Tridiagonal(dl, d, du)
4×4 Tridiagonal{Float64, Vector{Float64}}:
1.0 1.0 ⋅ ⋅
1.0 1.0 1.0 ⋅
⋅ 1.0 1.0 1.0
⋅ ⋅ 1.0 1.0
julia> X .*= 2
4×4 Tridiagonal{Float64, Vector{Float64}}:
2.0 2.0 ⋅ ⋅
2.0 2.0 2.0 ⋅
⋅ 2.0 2.0 2.0
⋅ ⋅ 2.0 2.0
julia> dl
3-element Vector{Float64}:
2.0
2.0
2.0
|
Doesn't that risk losing information from the wrapper? E.g. would |
Do you mean in the pseudo-code implementation of |
I meant this kind of method generally, but yes, I assume getting the ancestor through a SubArray would be broken in your example. |
Makes sense to me. |
Yes it is lossy and that is intentional. Let me summarize the functions first and then I can clarify why it being lossy is fine. Functions Introduced
struct MyVectorWrapper{T} <: AbstractVector{T}
v::Vector{T}
end
Base.parent(v::MyVectorWrapper) = v.v
z = Tridiagonal(MyVectorWrapper(...), ....) Calling
Why is lossy information still useful?Consider any array type that doesn't support If we detect that this is a special ancestor type (say TracedRArray), we do a fallback implementation where we materialize it into a dense array. While this is not going to be optimal for performance (in general), it is almost certainly not going to error. Note that in cases where a loop doesn't error, materializing a dense array will still be more performant than falling back to a loop.
I will open a draft PR on this. It might help us give correct results for https://github.com/JuliaArrays/ArrayInterface.jl/blob/master/ext/ArrayInterfaceGPUArraysCoreExt.jl#L8 for cases not covered in the union type |
It could also do |
Most details are in EnzymeAD/Reactant.jl#369 (comment). I will copy over the important parts.
We introduce a function
isa_wrapped_array
that downstream packages can use to mark that their array type wraps another array. Using a union type from Adapt doesn't solve this problem, because that fundamentally doesn't extend to new array types.With this function, we can override functions inside our custom interpreter. Consider this simple example of extending
LinearAlgebra.diag
The text was updated successfully, but these errors were encountered: