Skip to content
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

Apply new version of JuliaFormatter #589

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function DI.value_and_pullback(
rc = ruleconfig(backend)
y, pb = rrule_via_ad(rc, f, x, map(unwrap, contexts)...)
tx = map(ty) do dy
pb(dy)[2]
return pb(dy)[2]
end
return y, tx
end
Expand All @@ -50,7 +50,7 @@ function DI.value_and_pullback(
) where {C}
(; y, pb) = prep
tx = map(ty) do dy
pb(dy)[2]
return pb(dy)[2]
end
return copy(y), tx
end
Expand All @@ -65,7 +65,7 @@ function DI.pullback(
) where {C}
(; pb) = prep
tx = map(ty) do dy
pb(dy)[2]
return pb(dy)[2]
end
return tx
end
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function DI.pushforward(f, ::NoPushforwardPrep, ::AutoDiffractor, x, tx::NTuple)
# code copied from Diffractor.jl
z = ∂☆{1}()(ZeroBundle{1}(f), bundle(x, dx))
dy = z[TaylorTangentIndex(1)]
dy
return dy
end
return ty
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function batch_seeded_autodiff_thunk(
dinputs = only(reverse(f, args..., dresults_righttype, tape))
else
foreach(shadow_results, dresults) do d0, d
d0 .+= d # use recursive_add here?
return d0 .+= d # use recursive_add here?
end
dinputs = only(reverse(f, args..., tape))
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ to_val(::DI.BatchSizeSettings{B}) where {B} = Val(B)

## Annotations

function get_f_and_df(f::F, ::AutoEnzyme{M,Nothing}, ::Val{B}=Val(1)) where {F,M,B}
function get_f_and_df(f::F, ::AutoEnzyme{M,Nothing}; (::Val{B})=Val(1)) where {F,M,B}
Copy link
Member Author

Choose a reason for hiding this comment

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

this automatic formatting changes the semantics, turning a positional argument into a keyword argument

Copy link

Choose a reason for hiding this comment

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

this could be disabled with separate_kwargs_with_semicolon=false

I think this is the behaviour one would want for the option though, no? It seems if it didn't make this transformation previously it would be a bug.

Copy link

Choose a reason for hiding this comment

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

or is it such that ::Val{B}=Val(1) is not considered a keyword argument but val::Val{B}=Val(1) is?

Copy link
Member Author

@gdalle gdalle Oct 17, 2024

Choose a reason for hiding this comment

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

No, previously it wasn't a bug. It was an unnamed positional argument, for which I'm only interested in the type (think of it as a Holy trait).
Besides, @MilesCranmer seems to say that the same behavior can be observed for named positional arguments, which is also problematic.
Named or unnamed, positional arguments can also have default values, as long as they come at the end of the list of positional arguments and before the semicolon. They don't need to be keyword arguments to have default values.

Choose a reason for hiding this comment

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

Yes. Here’s an example in my codebase:

function check_constraints(
     tree::AbstractExpressionNode,
     options::AbstractOptions,
-    maxsize::Int,
+    maxsize::Int;
     cursize::Union{Int,Nothing}=nothing,
 )::Bool

This means that doing a function call check_constraints(tree, options, maxsize, cursize) will now throw an error when previously it worked.

return f
end

function get_f_and_df(f::F, ::AutoEnzyme{M,<:Const}, ::Val{B}=Val(1)) where {F,M,B}
function get_f_and_df(f::F, ::AutoEnzyme{M,<:Const}; (::Val{B})=Val(1)) where {F,M,B}
return Const(f)
end

Expand All @@ -30,8 +30,8 @@ function get_f_and_df(
EnzymeCore.DuplicatedNoNeed,
EnzymeCore.BatchDuplicatedNoNeed,
},
},
::Val{B}=Val(1),
};
(::Val{B})=Val(1),
) where {F,M,B}
# TODO: needs more sophistication for mixed activities
if B == 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function DI.pushforward(
)
ty = map(tx) do dx
v_vec = vcat(myvec(x), myvec(dx))
reshape(prep.jvp_exe(v_vec), size(y))
return reshape(prep.jvp_exe(v_vec), size(y))
end
return ty
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ function DI.pushforward(
) where {C}
step(t::Number, dx) = f(x .+ t .* dx, map(unwrap, contexts)...)
ty = map(tx) do dx
finite_difference_derivative(Base.Fix2(step, dx), zero(eltype(x)), fdtype(backend))
return finite_difference_derivative(
Base.Fix2(step, dx), zero(eltype(x)), fdtype(backend)
)
end
return ty
end
Expand All @@ -32,7 +34,7 @@ function DI.value_and_pushforward(
step(t::Number, dx) = f(x .+ t .* dx, map(unwrap, contexts)...)
y = f(x, map(unwrap, contexts)...)
ty = map(tx) do dx
finite_difference_derivative(
return finite_difference_derivative(
Base.Fix2(step, dx), zero(eltype(x)), fdtype(backend), eltype(y), y
)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function DI.value_and_pushforward(
return new_y
end
ty = map(tx) do dx
finite_difference_derivative(
return finite_difference_derivative(
Base.Fix2(step, dx), zero(eltype(x)), fdtype(backend), eltype(y), y
)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function DI.pushforward(
) where {C}
fc = with_contexts(f, contexts...)
ty = map(tx) do dx
jvp(backend.fdm, fc, (x, dx))
return jvp(backend.fdm, fc, (x, dx))
end
return ty
end
Expand Down Expand Up @@ -69,7 +69,7 @@ function DI.pullback(
) where {C}
fc = with_contexts(f, contexts...)
tx = map(ty) do dy
only(j′vp(backend.fdm, fc, dy, x))
return only(j′vp(backend.fdm, fc, dy, x))
end
return tx
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,13 @@ myderivative!(::Type{T}, dy, ydual) where {T} = dy .= myderivative.(T, ydual)

function mypartials(::Type{T}, ::Val{B}, ydual::Number) where {T,B}
return ntuple(Val(B)) do b
partials(T, ydual, b)
return partials(T, ydual, b)
end
end

function mypartials(::Type{T}, ::Val{B}, ydual) where {T,B}
return ntuple(Val(B)) do b
partials.(T, ydual, b)
return partials.(T, ydual, b)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function DI.value_and_pullback(
) where {C}
ys_and_tx = map(ty) do dy
y, tx = DI.value_and_pullback(f, prep, backend, x, (dy,), contexts...)
y, only(tx)
return y, only(tx)
end
y = first(ys_and_tx[1])
tx = last.(ys_and_tx)
Expand All @@ -91,7 +91,7 @@ function DI.value_and_pullback!(
) where {C}
ys = map(tx, ty) do dx, dy
y, _ = DI.value_and_pullback!(f, (dx,), prep, backend, x, (dy,), contexts...)
y
return y
end
y = ys[1]
return y, tx
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function DI.value_and_pullback(
) where {C}
tx = map(ty) do dy
_, tx = DI.value_and_pullback(f!, y, prep, backend, x, (dy,), contexts...)
only(tx)
return only(tx)
end
return y, tx
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function DI.value_and_pullback(
return dot(y_copy, dy)
end
tx = map(ty) do dy
gradient(Fix2(dotclosure, dy), x)
return gradient(Fix2(dotclosure, dy), x)
end
fc!(y, x)
return y, tx
Expand Down Expand Up @@ -70,7 +70,7 @@ function DI.pullback(
return dot(y_copy, dy)
end
tx = map(ty) do dy
gradient(Fix2(dotclosure, dy), x)
return gradient(Fix2(dotclosure, dy), x)
end
return tx
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function DI.pushforward(
)
ty = map(tx) do dx
v_vec = vcat(myvec(x), myvec(dx))
dy = prep.pf_exe(v_vec)
return dy = prep.pf_exe(v_vec)
end
return ty
end
Expand Down Expand Up @@ -302,7 +302,7 @@ function DI.hvp(f, prep::SymbolicsOneArgHVPPrep, ::AutoSymbolics, x, tx::NTuple)
return map(tx) do dx
v_vec = vcat(vec(x), vec(dx))
dg_vec = prep.hvp_exe(v_vec)
reshape(dg_vec, size(x))
return reshape(dg_vec, size(x))
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function DI.pushforward(
)
ty = map(tx) do dx
v_vec = vcat(myvec(x), myvec(dx))
dy = prep.pushforward_exe(v_vec)
return dy = prep.pushforward_exe(v_vec)
end
return ty
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function DI.value_and_pullback(
) where {C}
y, pb = forward(f, x, map(unwrap, contexts)...)
tx = map(ty) do dy
data(first(pb(dy)))
return data(first(pb(dy)))
end
return y, tx
end
Expand All @@ -49,7 +49,7 @@ function DI.value_and_pullback(
) where {C}
(; y, pb) = prep
tx = map(ty) do dy
data(first(pb(dy)))
return data(first(pb(dy)))
end
return copy(y), tx
end
Expand All @@ -64,7 +64,7 @@ function DI.pullback(
) where {C}
(; pb) = prep
tx = map(ty) do dy
data(first(pb(dy)))
return data(first(pb(dy)))
end
return tx
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function DI.value_and_pullback(
) where {C}
y, pb = pullback(f, x, map(unwrap, contexts)...)
tx = map(ty) do dy
first(pb(dy))
return first(pb(dy))
end
return y, tx
end
Expand All @@ -60,7 +60,7 @@ function DI.value_and_pullback(
) where {C}
(; y, pb) = prep
tx = map(ty) do dy
first(pb(dy))
return first(pb(dy))
end
return copy(y), tx
end
Expand All @@ -75,7 +75,7 @@ function DI.pullback(
) where {C}
(; pb) = prep
tx = map(ty) do dy
first(pb(dy))
return first(pb(dy))
end
return tx
end
Expand Down
4 changes: 2 additions & 2 deletions DifferentiationInterface/src/first_order/pullback.jl
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ function _pullback_via_pushforward(
) where {F,C}
dx = map(CartesianIndices(x)) do j
t1 = pushforward(f, pushforward_prep, backend, x, (basis(backend, x, j),), contexts...)
dot(dy, only(t1))
return dot(dy, only(t1))
end
return dx
end
Expand Down Expand Up @@ -270,7 +270,7 @@ function _pullback_via_pushforward(
t1 = pushforward(
f!, y, pushforward_prep, backend, x, (basis(backend, x, j),), contexts...
)
dot(dy, only(t1))
return dot(dy, only(t1))
end
return dx
end
Expand Down
4 changes: 2 additions & 2 deletions DifferentiationInterface/src/first_order/pushforward.jl
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ function _pushforward_via_pullback(
) where {F,C}
dy = map(CartesianIndices(y)) do i
t1 = pullback(f, pullback_prep, backend, x, (basis(backend, y, i),), contexts...)
dot(dx, only(t1))
return dot(dx, only(t1))
end
return dy
end
Expand Down Expand Up @@ -263,7 +263,7 @@ function _pushforward_via_pullback(
) where {F,C}
dy = map(CartesianIndices(y)) do i # preserve shape
t1 = pullback(f!, y, pullback_prep, backend, x, (basis(backend, y, i),), contexts...)
dot(dx, only(t1))
return dot(dx, only(t1))
end
return dy
end
Expand Down
2 changes: 1 addition & 1 deletion DifferentiationInterface/src/second_order/hvp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ function hvp(
(; outer_gradient_prep) = prep
rewrap = Rewrap(contexts...)
tg = map(tx) do dx
gradient(
return gradient(
shuffled_single_pushforward,
outer_gradient_prep,
outer(backend),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ LOGGING = get(ENV, "CI", "false") == "false"
function differentiatewith_scenarios()
bad_scens = # these closurified scenarios have mutation and type constraints
filter(default_scenarios(; include_normal=false, include_closurified=true)) do scen
DIT.function_place(scen) == :out
return DIT.function_place(scen) == :out
end
good_scens = map(bad_scens) do scen
DIT.change_function(scen, DifferentiateWith(scen.f, AutoFiniteDiff()))
return DIT.change_function(scen, DifferentiateWith(scen.f, AutoFiniteDiff()))
end
return good_scens
end
Expand Down
2 changes: 1 addition & 1 deletion DifferentiationInterface/test/Misc/ZeroBackends/test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ test_differentiation(
)

test_differentiation(
AutoSparse.(zero_backends, coloring_algorithm=GreedyColoringAlgorithm()),
AutoSparse.(zero_backends; coloring_algorithm=GreedyColoringAlgorithm()),
default_scenarios(; include_constantified=true);
correctness=false,
type_stability=:full,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function comp_to_num(x::ComponentVector)::Number
return sum(sin.(x.a)) + sum(cos.(x.b))
end

comp_to_num_gradient(x) = ComponentVector(; a=cos.(x.a), b=-sin.(x.b))
comp_to_num_gradient(x) = ComponentVector(; a=cos.(x.a), b=(-sin.(x.b)))

function comp_to_num_pushforward(x, dx)
g = comp_to_num_gradient(x)
Expand Down
4 changes: 2 additions & 2 deletions DifferentiationInterfaceTest/src/scenarios/sparse.jl
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ end

function diffsquarecube_matvec!(y::AbstractVector, x::AbstractMatrix)
m, n = size(x)
diffsquare!(view(y, 1:(m * n - 1)), vec(x))
diffcube!(view(y, (m * n):(2(m * n) - 2)), vec(x))
diffsquare!(view(y, 1:(m*n-1)), vec(x))
diffcube!(view(y, (m*n):(2(m*n)-2)), vec(x))
return nothing
end

Expand Down
Loading
Loading