-
Notifications
You must be signed in to change notification settings - Fork 16
Loosen types for Reactant #162
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
Changes from 12 commits
1b1308b
ef2c0d8
9ffba74
593f623
eceb64e
b0c579d
9ec1c9a
7d1d756
40b7d33
9f8fd8c
0ddba3d
8a54315
9b6e831
5f368e8
4d74ff2
962db49
9ee12fd
5f7a0a4
7e6352b
bab0a40
070c448
a5f1e40
f5173e6
235062f
b401cea
480917c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| module ReactantExt | ||
| using TransformVariables: TransformVariables, ArrayTransformation, LogJacFlag, | ||
| logjac_zero, transform_with, _ensure_float, dimension | ||
| using Reactant | ||
| using Reactant: TracedRNumber, AnyTracedRArray | ||
|
|
||
| RInt = Union{Int, TracedRNumber{Int}} | ||
ptiede marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Base.@propagate_inbounds function TransformVariables.tv_getindex(a::AnyTracedRArray, i::RInt) | ||
| Reactant.@allowscalar a[i] | ||
| end | ||
|
|
||
| # The dims of the ArrayTransformation must be constant because Reactant can only deal with non-dynamic arrays. | ||
| Base.@nospecialize function Reactant.traced_type_inner( | ||
| @nospecialize(prev::Type{ArrayTransformation{T, M}}), | ||
| seen, | ||
| @nospecialize(mode::TraceMode), | ||
| @nospecialize(track_numbers::Type), | ||
| @nospecialize(sharding), | ||
| @nospecialize(runtime) | ||
| ) where {T, M} | ||
| T_traced = Reactant.traced_type_inner(T, seen, mode, track_numbers, sharding, runtime) | ||
| return TransformVariables.ArrayTransformation{T_traced, M} | ||
| end | ||
|
|
||
| TransformVariables._ensure_float(x::Type{T}) where {T<:TracedRNumber} = T | ||
ptiede marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| @noinline function TransformVariables.transform_with( | ||
| flag::TransformVariables.LogJacFlag, | ||
| transformation::TransformVariables.ArrayTransformation, | ||
| x::AnyTracedRArray, | ||
| index | ||
| ) | ||
| (; inner_transformation, dims) = transformation | ||
| # NOTE not using index increments as that somehow breaks type inference | ||
| d = dimension(inner_transformation) # length of an element transformation | ||
| len = prod(dims) # number of elements | ||
| 𝐼 = range(index; length = len, step = d) | ||
|
|
||
| # # Reactant can't easily handle the usual return type because the array eltype is not a Reacant primitive | ||
ptiede marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # # so we have to do a 2-pass algorithm | ||
| # yℓ = map(index -> ((y, ℓ, _) = transform_with(flag, inner_transformation, x, index); y), 𝐼) | ||
| # ℓa = map(index -> ((y, ℓ, _) = transform_with(flag, inner_transformation, x, index); ℓ), 𝐼) | ||
|
|
||
| # index′ = index + d * len | ||
| # y = reshape(yℓ, dims) | ||
| # ℓa = sum(ℓa) | ||
| # y, ℓa, index′ | ||
|
|
||
| tmp,_,_ = transform_with(flag, inner_transformation, x, first(𝐼)) | ||
| ℓa = logjac_zero(flag, _ensure_float(eltype(x))) | ||
| if typeof(tmp) <: Number | ||
ptiede marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| yℓ = similar(x, typeof(tmp), length(𝐼)) | ||
| elseif typeof(tmp) <: AbstractArray | ||
ptiede marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # convert to a larger array since reactant can't easily index into an julia array of Reactant arrays with a traced number. | ||
| yℓ = similar(tmp, size(tmp)..., length(𝐼)) | ||
| else | ||
| throw(ArgumentError("Number and AbstractArray transformations are only supported in Reactant compilation mode")) | ||
ptiede marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| end | ||
| @trace track_numbers=false for i in eachindex(𝐼) | ||
| idx = 𝐼[i] | ||
| y, ℓ, _ = transform_with(flag, inner_transformation, x, idx) | ||
| if !isempty(y) | ||
| ℓa += ℓ | ||
| end | ||
| if tmp isa Number | ||
| Reactant.@allowscalar yℓ[i] = y | ||
| else | ||
| idxs = ntuple(n-> Colon(), ndims(tmp)) | ||
| yℓ[idxs..., i] = y | ||
| end | ||
| end | ||
|
|
||
| if tmp isa AbstractArray | ||
| yℓ0 = collect(eachslice(yℓ, dims=ndims(yℓ))) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wsmoses if I change this to [yℓ[ntuple(n->Colon(), ndims(tmp))..., i] for i in axes(yℓ, ndims(yℓ)]Reactant gives the wrong answer. I'll see if I can cook up a MWE for this but likely some pass is strange.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wsmoses can you please comment on this? |
||
| else | ||
| yℓ0 = yℓ | ||
| end | ||
|
|
||
| index′ = index + d * len | ||
| reshape(yℓ0, dims), ℓa, index′ | ||
| end | ||
|
|
||
| # To prevent ambiguities with regular method | ||
| function TransformVariables.transform_with(flag::LogJacFlag, t::ArrayTransformation{TransformVariables.Identity}, x::AnyTracedRArray, index) | ||
| index′ = index+dimension(t) | ||
| inds = Reactant.TracedUnitRange(index, index′-1, dimension(t)) | ||
| y = reshape(x[inds], t.dims) | ||
| y, logjac_zero(flag, _ensure_float(eltype(x))), index′ | ||
| end | ||
|
|
||
|
|
||
| end | ||
ptiede marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,12 +17,12 @@ abstract type ScalarTransform <: AbstractTransform end | |
|
|
||
| dimension(::ScalarTransform) = 1 | ||
|
|
||
| function transform_with(flag::NoLogJac, t::ScalarTransform, x::AbstractVector, index::Int) | ||
| transform(t, @inbounds x[index]), flag, index + 1 | ||
| function transform_with(flag::NoLogJac, t::ScalarTransform, x::AbstractVector, index) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a bit surprised that you need this, since Can you explain what the actual type is that you need here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sometimes in Reactant you can get a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specifically for the |
||
| transform(t, @inbounds tv_getindex(x, index)), flag, index + 1 | ||
| end | ||
|
|
||
| function transform_with(::LogJac, t::ScalarTransform, x::AbstractVector, index::Int) | ||
| transform_and_logjac(t, @inbounds x[index])..., index + 1 | ||
| function transform_with(::LogJac, t::ScalarTransform, x::AbstractVector, index) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
| transform_and_logjac(t, @inbounds tv_getindex(x, index))..., index + 1 | ||
| end | ||
|
|
||
| function inverse_at!(x::AbstractVector, index::Int, t::ScalarTransform, y) | ||
|
|
@@ -43,15 +43,15 @@ Identity ``x ↦ x``. | |
| """ | ||
| struct Identity <: ScalarTransform end | ||
|
|
||
| transform(::Identity, x::Real) = x | ||
| transform(::Identity, x::Number) = x | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In principle I am OK with widening
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ya I agree it is very sad. |
||
|
|
||
| transform_and_logjac(::Identity, x::Real) = x, logjac_zero(LogJac(), typeof(x)) | ||
| transform_and_logjac(::Identity, x::Number) = x, logjac_zero(LogJac(), typeof(x)) | ||
|
|
||
| inverse_eltype(t::Identity, ::Type{T}) where T = T | ||
|
|
||
| inverse(::Identity, x::Number) = x | ||
|
|
||
| inverse_and_logjac(::Identity, x::Real) = x, logjac_zero(LogJac(), typeof(x)) | ||
| inverse_and_logjac(::Identity, x::Number) = x, logjac_zero(LogJac(), typeof(x)) | ||
|
|
||
| #### | ||
| #### elementary scalar transforms | ||
|
|
@@ -64,9 +64,9 @@ Exponential transformation `x ↦ eˣ`. Maps from all reals to the positive real | |
| """ | ||
| struct TVExp <: ScalarTransform end | ||
|
|
||
| transform(::TVExp, x::Real) = exp(x) | ||
| transform(::TVExp, x::Number) = exp(x) | ||
|
|
||
| transform_and_logjac(t::TVExp, x::Real) = transform(t, x), x | ||
| transform_and_logjac(t::TVExp, x::Number) = transform(t, x), x | ||
|
|
||
| inverse_eltype(t::TVExp, ::Type{T}) where T = _ensure_float(T) | ||
|
|
||
|
|
@@ -83,9 +83,9 @@ Logistic transformation `x ↦ logit(x)`. Maps from all reals to (0, 1). | |
| """ | ||
| struct TVLogistic <: ScalarTransform end | ||
|
|
||
| transform(::TVLogistic, x::Real) = logistic(x) | ||
| transform(::TVLogistic, x::Number) = logistic(x) | ||
|
|
||
| transform_and_logjac(t::TVLogistic, x::Real) = transform(t, x), logistic_logjac(x) | ||
| transform_and_logjac(t::TVLogistic, x::Number) = transform(t, x), logistic_logjac(x) | ||
|
|
||
| inverse_eltype(t::TVLogistic, ::Type{T}) where T = _ensure_float(T) | ||
|
|
||
|
|
@@ -100,13 +100,13 @@ $(TYPEDEF) | |
|
|
||
| Shift transformation `x ↦ x + shift`. | ||
| """ | ||
| struct TVShift{T <: Real} <: ScalarTransform | ||
| struct TVShift{T} <: ScalarTransform | ||
| shift::T | ||
| end | ||
|
|
||
| transform(t::TVShift, x::Real) = x + t.shift | ||
| transform(t::TVShift, x::Number) = x + t.shift | ||
|
|
||
| transform_and_logjac(t::TVShift, x::Real) = transform(t, x), logjac_zero(LogJac(), typeof(x)) | ||
| transform_and_logjac(t::TVShift, x::Number) = transform(t, x), logjac_zero(LogJac(), typeof(x)) | ||
|
|
||
| inverse_eltype(t::TVShift{S}, ::Type{T}) where {S,T} = typeof(zero(_ensure_float(T)) - zero(S)) | ||
|
|
||
|
|
@@ -129,15 +129,15 @@ end | |
|
|
||
| TVScale(scale::T) where {T} = TVScale{T}(scale) | ||
|
|
||
| transform(t::TVScale, x::Real) = t.scale * x | ||
| transform(t::TVScale, x::Number) = t.scale * x | ||
|
|
||
| transform_and_logjac(t::TVScale{<:Real}, x::Real) = transform(t, x), log(t.scale) | ||
| transform_and_logjac(t::TVScale{<:Real}, x::Number) = transform(t, x), log(t.scale) | ||
|
|
||
| inverse_eltype(t::TVScale{S}, ::Type{T}) where {S,T} = typeof(oneunit(T) / oneunit(S)) | ||
|
|
||
| inverse(t::TVScale, x::Number) = x / t.scale | ||
|
|
||
| inverse_and_logjac(t::TVScale{<:Real}, x::Number) = inverse(t, x), -log(t.scale) | ||
| inverse_and_logjac(t::TVScale, x::Number) = inverse(t, x), -log(t.scale) | ||
|
|
||
| """ | ||
| $(TYPEDEF) | ||
|
|
@@ -147,8 +147,8 @@ Negative transformation `x ↦ -x`. | |
| struct TVNeg <: ScalarTransform | ||
| end | ||
|
|
||
| transform(::TVNeg, x::Real) = -x | ||
| transform_and_logjac(t::TVNeg, x::Real) = transform(t, x), logjac_zero(LogJac(), typeof(x)) | ||
| transform(::TVNeg, x::Number) = -x | ||
| transform_and_logjac(t::TVNeg, x::Number) = transform(t, x), logjac_zero(LogJac(), typeof(x)) | ||
|
|
||
| inverse_eltype(::TVNeg, ::Type{T}) where T = typeof(-oneunit(T)) | ||
| inverse(::TVNeg, x::Number) = -x | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,23 @@ | |
| ### logistic and logit | ||
| ### | ||
|
|
||
| function logistic_logjac(x::Real) | ||
|
|
||
ptiede marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| """ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it actually necessary to add a proper doctoring to an internal method? Should it just be a comment?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a tendency to add docstrings to everything, so I think it is fine. |
||
| $(SIGNATURES) | ||
| Method used in place of `Base.getindex` within this package. | ||
ptiede marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| When `a <: Reactant.AnyTracedRArray` this adds a `@allowscalar` annotation so that the function can | ||
| be compiled with Reactant. When `a` is not a `Reactant.AnyTracedRArray` it simply returns `a[i]`. | ||
|
|
||
| !!! warn | ||
| This is necessary because by default Reactant does not allow scalar indexing of arrays unless you | ||
| opt in. Note that often `Reactant` is able to raise the scalar indexing to the level of the whole | ||
| array so this operation is not necessarily slow, but there are no guarantees. | ||
| """ | ||
| Base.@propagate_inbounds function tv_getindex(a, i) | ||
| return a[i] | ||
| end | ||
|
|
||
| function logistic_logjac(x::Number) | ||
| mx = -abs(x) | ||
| mx - 2*log1pexp(mx) | ||
| end | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.