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.

Sign up for GitHub

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

Jump to bottom

effectsSpeedup #170

Merged
merged 6 commits into from
Feb 21, 2024
Merged

effectsSpeedup #170

merged 6 commits into from
Feb 21, 2024

Conversation

behinger
Copy link
Member

  • speedup unnecessary vcat
  • fix CI after UnfoldSim breaking
  • fix further UnfoldSim.jl update bug
  • fix io test

github-actions[bot]
github-actions bot reviewed Feb 20, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

JuliaFormatter

test/fit_LMM.jl|81|
test/fit_LMM.jl|93|
test/fit_LMM.jl|108|
test/fit_LMM.jl|121|
test/fit_LMM.jl|131|
test/fit_LMM.jl|138|
test/fit_LMM.jl|142|
test/fit_LMM.jl|152|
test/fit_LMM.jl|164|
test/fit_LMM.jl|176|
test/fit_LMM.jl|185|
test/fit_LMM.jl|191|
test/fit_LMM.jl|207|
test/fit_LMM.jl|223|
test/fit_LMM.jl|231|
test/io.jl|3|
test/io.jl|12|
test/io.jl|16|
test/io.jl|32|
test/io.jl|34|
test/io.jl|39|
test/io.jl|41|
test/io.jl|65|
test/io.jl|79|
test/io.jl|86|
test/io.jl|95|
test/io.jl|100|
test/io.jl|102|
test/io.jl|123|

src/effects.jl
@@ -26,7 +26,7 @@ Calculates marginal effects for all term-combinations in `design`.
"""


function effects(design::AbstractDict, model::UnfoldModel; typical = mean)
function effects(design::AbstractDict, model::UnfoldModel; typical=mean)
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
function effects(design::AbstractDict, model::UnfoldModel; typical=mean)
function effects(design::AbstractDict, model::UnfoldModel; typical = mean)

src/effects.jl
@@ -49,7 +49,7 @@ function effects(design::AbstractDict, model::UnfoldModel; typical = mean)
bnames = repeat.(bnames, [e.stop + e.step - 1 for e in eff[1]])

result = DataFrame(
cast_referenceGrid(reference_grid, eff[3], eff[2]; basisname = vcat(bnames...)),
cast_referenceGrid(reference_grid, eff[3], eff[2]; basisname=vcat(bnames...)),
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
cast_referenceGrid(reference_grid, eff[3], eff[2]; basisname=vcat(bnames...)),
cast_referenceGrid(reference_grid, eff[3], eff[2]; basisname = vcat(bnames...)),

src/effects.jl
@@ -96,21 +96,21 @@ function _typify(
tmpf = FormulaTerm(tmpf.lhs, tmpf.rhs.term)

# typify that
tmpf = typify(reference_grid, tmpf, m[f]; typical = typical)
tmpf = typify(reference_grid, tmpf, m[f]; typical=typical)
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
tmpf = typify(reference_grid, tmpf, m[f]; typical=typical)
tmpf = typify(reference_grid, tmpf, m[f]; typical = typical)

src/effects.jl

# regenerate TimeExpansion
tmpf = Unfold.TimeExpandedTerm(
tmpf,
form[f].rhs.basisfunction;
eventfields = form[f].rhs.eventfields,
eventfields=form[f].rhs.eventfields,
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
eventfields=form[f].rhs.eventfields,
eventfields = form[f].rhs.eventfields,

src/effects.jl
)
form_typical[f] = tmpf
end
return form_typical
end
function _typify(reference_grid, form::FormulaTerm, m, typical)

return [typify(reference_grid, form, m; typical = typical)]
return [typify(reference_grid, form, m; typical=typical)]
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
return [typify(reference_grid, form, m; typical=typical)]
return [typify(reference_grid, form, m; typical = typical)]

test/fit_LMM.jl
)
df = Unfold.coeftable(m_mum)
@test isapprox(
df[(df.channel.==1).&(df.coefname.=="condA: 1").&(df.time.==0.0), :estimate],
[5.618, 9.175],
rtol = 0.1,
rtol=0.1,
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
rtol=0.1,
rtol = 0.1,

test/fit_LMM.jl
@@ -53,32 +53,32 @@
evts_missing_e,
data_missing_e,
times,
contrasts = Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
contrasts=Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
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
contrasts=Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
contrasts = Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),

test/fit_LMM.jl
)
df = coeftable(m_mum)
@test isapprox(
df[(df.channel.==1).&(df.coefname.=="condA: 1").&(df.time.==0.0), :estimate],
[5.618, 9.175],
rtol = 0.1,
rtol=0.1,
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
rtol=0.1,
rtol = 0.1,

test/fit_LMM.jl
)


# Timexpanded Univariate Mixed
f = @formula 0 ~ 1 + condA + condB + (1 + condA | subject)
basisfunction = firbasis(τ = (-0.2, 0.3), sfreq = 10, name = "ABC")
basisfunction = firbasis(τ=(-0.2, 0.3), sfreq=10, name="ABC")
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
basisfunction = firbasis(τ=(-0.2, 0.3), sfreq=10, name="ABC")
basisfunction = firbasis(τ = (-0.2, 0.3), sfreq = 10, name = "ABC")

test/fit_LMM.jl
@time m_tum = fit(
UnfoldModel,
f,
evts,
data,
basisfunction,
contrasts = Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
contrasts=Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
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
contrasts=Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
contrasts = Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),

github-actions[bot]
github-actions bot reviewed Feb 20, 2024
View reviewed changes
test/fit_LMM.jl
)
df = coeftable(m_tum)
@test isapprox(
df[(df.channel.==1).&(df.coefname.=="condA: 1").&(df.time.==0.0), :estimate],
[5.618, 9.175],
rtol = 0.1,
rtol=0.1,
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
rtol=0.1,
rtol = 0.1,

test/fit_LMM.jl
@@ -90,7 +90,7 @@
evts,
data_missing,
basisfunction,
contrasts = Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
contrasts=Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
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
contrasts=Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),
contrasts = Dict(:condA => EffectsCoding(), :condB => EffectsCoding()),

test/fit_LMM.jl
Comment on lines +108 to +109
b1 = firbasis(τ=(-0.2, 0.3), sfreq=10, name="A")
b2 = firbasis(τ=(-0.1, 0.3), sfreq=10, name="B")
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
b1 = firbasis(τ=(-0.2, 0.3), sfreq=10, name="A")
b2 = firbasis(τ=(-0.1, 0.3), sfreq=10, name="B")
b1 = firbasis(τ = (-0.2, 0.3), sfreq = 10, name = "A")
b2 = firbasis(τ = (-0.1, 0.3), sfreq = 10, name = "B")

test/fit_LMM.jl
@@ -118,7 +118,7 @@
@test isapprox(
df[(df.channel.==1).&(df.coefname.=="condB").&(df.time.==0.0), :estimate],
[18.21, 17.69],
rtol = 0.1,
rtol=0.1,
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
rtol=0.1,
rtol = 0.1,

test/fit_LMM.jl
@@ -128,19 +128,19 @@
Dict(0 => (f1_lmm, b1), 1 => (f2_lmm, b2)),
evts,
data,
eventcolumn = "condA",
eventcolumn="condA",
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
eventcolumn="condA",
eventcolumn = "condA",

test/io.jl

# Define a model formula with interaction term and random effects for subjects
f2 = @formula(0 ~ 1 + A * B + (1 | subject));
τ2 = [-0.1, 1];
sfreq2 = 100;
times2 = range(τ2[1], length = size(data2, 1), step = 1 ./ sfreq2);
times2 = range(τ2[1], length=size(data2, 1), step=1 ./ sfreq2);
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
times2 = range(τ2[1], length=size(data2, 1), step=1 ./ sfreq2);
times2 = range(τ2[1], length = size(data2, 1), step = 1 ./ sfreq2);

test/io.jl
Comment on lines +95 to +96
save(joinpath(save_path, "m2_compressed.jld2"), m2; compress=true)
m2_loaded = load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs=true)
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
save(joinpath(save_path, "m2_compressed.jld2"), m2; compress=true)
m2_loaded = load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs=true)
save(joinpath(save_path, "m2_compressed.jld2"), m2; compress = true)
m2_loaded = load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs = true)

test/io.jl

@testset "2x2 MultiSubjectDesign Mixed-effects model" begin
# save the model to a compressed .jld2 file and load it again
save(joinpath(save_path, "m2_compressed.jld2"), m2; compress = true)
save(joinpath(save_path, "m2_compressed.jld2"), m2; compress=true)
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
save(joinpath(save_path, "m2_compressed.jld2"), m2; compress=true)
save(joinpath(save_path, "m2_compressed.jld2"), m2; compress = true)

test/io.jl
m2_loaded =
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs = true)
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs=true)
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
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs=true)
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs = true)

test/io.jl
@@ -119,7 +120,7 @@ m2_loaded = load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generat

# load the model without reconstructing the designmatrix
m2_loaded_without_dm =
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs = false)
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs=false)
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
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs=false)
load(joinpath(save_path, "m2_compressed.jld2"), UnfoldModel, generate_Xs = false)

@behinger behinger merged commit 55803b0 into main Feb 21, 2024
5 of 6 checks passed
@behinger behinger deleted the effectsSpeedup branch March 19, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@behinger