feat: Preserve Indices When Copying Tracked Arrays #263
Conversation
DhairyaLGandhi
changed the title
|
Bumping :) |
| return T1.(x) | ||
| end | ||
| function Base.convert(::Type{ComponentArray{T,N,A1,Ax1}}, x::ComponentArray{T,N,A2,Ax2}) where {T,N,A1,A2,Ax1,Ax2} | ||
| return x |
There was a problem hiding this comment.
Wouldn't we want the axes to be from the type we're converting to? I think part of the contract of convert is that the output needs to be of the type you're converting to.
There was a problem hiding this comment.
It follows the same pattern as some of the other dispatches https://github.com/jonniedie/ComponentArrays.jl/blob/554a9c03373680af84586762f68ebd32b6d34abe/src/similar_convert_copy.jl#L52
There was a problem hiding this comment.
I think in those dispatches we'd want to just have Ax instead of Ax1 and Ax2 too
There was a problem hiding this comment.
Agreed! I do not know if there are downstream packages relying on that behaviour, so I will not touch those dispatches in this PR. For now, I have matched the axes for this dispatch
There was a problem hiding this comment.
Oh, I definitely need to change those, then. But that doesn't block this PR.
There was a problem hiding this comment.
I have also had to add another trivial dispatch to avoid ambiguities.
|
Sorry, I commented this morning but forgot to hit "Submit Review" |
|
Bump :) |
3 participants
Ref failures in https://github.com/SciML/SciMLSensitivity.jl/actions/runs/9669459445/job/26676007518?pr=1057