avoid using `threadid` in landau example, instead use OhMyThreads + ChunkSplitters by KristofferC · Pull Request #1294 · Ferrite-FEM/Ferrite.jl

KristofferC · 2026-03-05T11:32:52Z

Indexing by threadid is not really valid. I removed the calcall because it felt kind of pointless to just have hanging there.

codecov · 2026-03-05T11:35:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.25%. Comparing base (236eb50) to head (36300d5).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1294   +/-   ##
=======================================
  Coverage   94.25%   94.25%           
=======================================
  Files          40       40           
  Lines        6750     6750           
=======================================
  Hits         6362     6362           
  Misses        388      388

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…hunkSplitters

KnutAM

Nice 🚀 Added some more name-fixes thread->task and colors (I guess colored_indices is better but a bit long...)

I don't know all the OhMyThreads syntax, but looks good to me and nice to get rid of the @assemble macro!

I think adding a quick test would be nice at the end though

KnutAM · 2026-03-05T20:51:31Z

@@ -47,7 +48,7 @@ struct ModelParams{V, T}
 end

 # ### ThreadCache


Suggested change

# ### ThreadCache

# ### TaskCache

KnutAM · 2026-03-05T20:51:45Z

 # ### ThreadCache
-# This holds the values that each thread will use during the assembly.
+# This holds the values that each task will use during the assembly.
 struct ThreadCache{CV, T, DIM, F <: Function, GC <: GradientConfig, HC <: HessianConfig}


Suggested change

struct ThreadCache{CV, T, DIM, F <: Function, GC <: GradientConfig, HC <: HessianConfig}

struct TaskCache{CV, T, DIM, F <: Function, GC <: GradientConfig, HC <: HessianConfig}

KnutAM · 2026-03-05T20:51:56Z

    dofs::Vector{T}
    dofhandler::DH
    boundaryconds::CH
    threadindices::Vector{Vector{Int}}


Suggested change

threadindices::Vector{Vector{Int}}

colors::Vector{Vector{Int}}

KnutAM · 2026-03-05T20:52:13Z

-function LandauModel(α, G, gridsize, left::Vec{DIM, T}, right::Vec{DIM, T}, elpotential) where {DIM, T}
+function LandauModel(α, G, gridsize, left::Vec{DIM, T}, right::Vec{DIM, T}, elpotential, ntasks) where {DIM, T}
    grid = generate_grid(Tetrahedron, gridsize, left, right)
    threadindices = Ferrite.create_coloring(grid)


Suggested change

threadindices = Ferrite.create_coloring(grid)

colors = create_coloring(grid)

KnutAM · 2026-03-05T20:53:05Z

    cpc = length(grid.cells[1].nodes)
-    caches = [ThreadCache(dpc, cpc, copy(cvP), ModelParams(α, G), elpotential) for t in 1:Threads.maxthreadid()]
+    caches = [ThreadCache(dpc, cpc, copy(cvP), ModelParams(α, G), elpotential) for _ in 1:ntasks]
    return LandauModel(dofvector, dofhandler, boundaryconds, threadindices, caches)


Suggested change

return LandauModel(dofvector, dofhandler, boundaryconds, threadindices, caches)

return LandauModel(dofvector, dofhandler, boundaryconds, colors, caches)

KnutAM · 2026-03-05T20:55:50Z

-# everything is combined into a model.
+# Everything is combined into a model. The caches are pre-allocated (one per task)
+# and indexed by chunk index during assembly.
 mutable struct LandauModel{T, DH <: DofHandler, CH <: ConstraintHandler, TC <: ThreadCache}


Suggested change

mutable struct LandauModel{T, DH <: DofHandler, CH <: ConstraintHandler, TC <: ThreadCache}

mutable struct LandauModel{T, DH <: DofHandler, CH <: ConstraintHandler, TC <: TaskCache}

KnutAM · 2026-03-05T20:56:41Z

Suggested change

function TaskCache(dpc::Int, nodespercell, cvP::CellValues, modelparams, elpotential)

KnutAM · 2026-03-05T20:56:51Z

Suggested change

return TaskCache(cvP, element_indices, element_dofs, element_gradient, element_hessian, element_coords, potfunc, gradconf, hessconf)

KnutAM · 2026-03-05T20:58:46Z

+    out = zero(T)
+    for indices in model.threadindices
+        partial = OhMyThreads.@tasks for (ichunk, range) in enumerate(chunks(indices; n = length(model.caches)))
+            @set reducer = +


Would this make sense to be extra clear where this macro comes from? (I assume it is from here)

Suggested change

@set reducer = +

OhMyThreads.@set reducer = +

KnutAM · 2026-03-05T21:09:45Z

 right = Vec{3}((75.0, 25.0, 2.0))
-model = LandauModel(α, G, (50, 50, 2), left, right, element_potential)
+model = LandauModel(α, G, (50, 50, 2), left, right, element_potential, Threads.nthreads())



Would be nice to add some quick tests to check that we don't make unintended changes (both here and for future)?

Suggested change

dh = model.dofhandler #hide

ddf = allocate_matrix(dh) #hide

df = zeros(ndofs(dh)) #hide

a = collect(range(0, 1, ndofs(dh))) #hide

@test F(a, model) ≈ ?? #hide

∇F!(df, a, model) #hide

@test norm(df) ≈ ?? #hide

∇²F!(ddf, a, model) #hide

@test norm(ddf) ≈ ?? #hide

KristofferC · 2026-03-06T11:37:38Z

Updated based on review comments. We will see if the result test passes everywhere.

termi-official

Other than one more note this PR is good from my side.

termi-official · 2026-03-06T14:17:58Z

 save_landau("landaufinal", model)

+using Test # src
+@test Optim.minimum(res) ≈ -10858.806775 # src


Isn't the tolerance here a bit tight? I.e. can we really guarantee machine precision, such that in the future we won't see failures here popping up due to changes in Ferrite (or e.g. changes in Optim.jl).

This is using approx so not machine precision? I already cut off a bunch of decimals here from the answer I got locally.

I guess the approx tolerance is quite tight for an optimization run (which is why I originally suggested just to test the different assembly runs), but I'm also fine merging this as is and then adopt the tests if we see random failures in the future (should be fine see manually that it fails with small changes now that we have a reference solution). Of course, we could do

Suggested change

@test Optim.minimum(res) ≈ -10858.806775 # src

@test Optim.minimum(res) ≈ -10858.807f0 # src

to reduce the precision 😄

We can mess with it if we see there is an actual problem in the future?

KristofferC requested review from KnutAM, fredrikekre and termi-official March 5, 2026 11:32

avoid using threadid in landau example, instead use OhMyThreads + C…

c98643a

…hunkSplitters

KristofferC force-pushed the kc/landau_chunk branch from 69f8150 to c98643a Compare March 5, 2026 11:37

KnutAM reviewed Mar 5, 2026

View reviewed changes

address review

36300d5

termi-official reviewed Mar 6, 2026

View reviewed changes

Comment thread docs/src/literate-gallery/landau.jl

termi-official approved these changes Mar 6, 2026

View reviewed changes

KnutAM approved these changes Mar 6, 2026

View reviewed changes

KnutAM merged commit eb76ddc into master Mar 6, 2026
15 of 16 checks passed

KnutAM deleted the kc/landau_chunk branch March 6, 2026 19:52

		@@ -47,7 +48,7 @@ struct ModelParams{V, T}
		end

		# ### ThreadCache

	struct ThreadCache{CV, T, DIM, F <: Function, GC <: GradientConfig, HC <: HessianConfig}
	struct TaskCache{CV, T, DIM, F <: Function, GC <: GradientConfig, HC <: HessianConfig}

	threadindices::Vector{Vector{Int}}
	colors::Vector{Vector{Int}}

	threadindices = Ferrite.create_coloring(grid)
	colors = create_coloring(grid)

	return LandauModel(dofvector, dofhandler, boundaryconds, threadindices, caches)
	return LandauModel(dofvector, dofhandler, boundaryconds, colors, caches)

	mutable struct LandauModel{T, DH <: DofHandler, CH <: ConstraintHandler, TC <: ThreadCache}
	mutable struct LandauModel{T, DH <: DofHandler, CH <: ConstraintHandler, TC <: TaskCache}


	function TaskCache(dpc::Int, nodespercell, cvP::CellValues, modelparams, elpotential)


	return TaskCache(cvP, element_indices, element_dofs, element_gradient, element_hessian, element_coords, potfunc, gradconf, hessconf)

+dh = model.dofhandler               #hide
+ddf = allocate_matrix(dh)           #hide
+df = zeros(ndofs(dh))               #hide
+a = collect(range(0, 1, ndofs(dh))) #hide
+@test F(a, model) ≈ ??              #hide
+∇F!(df, a, model)                   #hide
+@test norm(df) ≈ ??                 #hide
+∇²F!(ddf, a, model)                 #hide
+@test norm(ddf) ≈ ??                #hide

	@test Optim.minimum(res) ≈ -10858.806775 # src
	@test Optim.minimum(res) ≈ -10858.807f0 # src

Conversation

KristofferC commented Mar 5, 2026

Uh oh!

codecov Bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

KnutAM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KristofferC commented Mar 6, 2026

Uh oh!

Uh oh!

termi-official left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Mar 5, 2026 •

edited

Loading