Skip to content

Conversation

@wsmoses
Copy link
Member

@wsmoses wsmoses commented Jan 3, 2025

No description provided.

@wsmoses wsmoses requested a review from avik-pal January 3, 2025 19:52
Comment on lines 707 to 709
@inline function to_rarray_internal(
@nospecialize(x::AbstractArray{<:ReactantPrimitive}), ::Tuple
@nospecialize(x::Array{<:ReactantPrimitive}), ::Tuple
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@inline function to_rarray_internal(
@nospecialize(x::AbstractArray{<:ReactantPrimitive}), ::Tuple
@nospecialize(x::Array{<:ReactantPrimitive}), ::Tuple
)
@inline function to_rarray_internal(@nospecialize(x::Array{<:ReactantPrimitive}), ::Tuple)

function Reactant.traced_type(
::Type{<:OffsetArray{<:Any,N, T}}, seen::ST, ::Val{mode}, track_numbers
) where {T, N,ST,mode}
T2 = Reactant.traced_type(T, seen, Val(mode), track_numbers)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avik-pal so the reason this is necessary here is because the default conversion of ConcreteRArray breaks this where AFAICT OffsetArray is OffsetArray{ElType, Dim, ArrayType} which for a concreterarray is OffsetArray{Float64, 2, ConcreRMatrix{Float64}}. however the conversion of a TracedRArray is OffsetArray{TracedRNumber{Float64}, 2, TracedRMatrix{Float64}}.

The following code should always work, however.

wsmoses and others added 2 commits January 3, 2025 14:55
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@wsmoses wsmoses merged commit 0e764de into main Jan 3, 2025
30 of 39 checks passed
@wsmoses wsmoses deleted the oa branch January 3, 2025 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants