WIP add task_local for duplication things for multithreading by fredrikekre · Pull Request #1070 · Ferrite-FEM/Ferrite.jl

fredrikekre · 2024-09-26T09:54:22Z

No description provided.

KnutAM · 2024-09-26T10:46:38Z

I don't have time to look in detail, but I really like to have this interface!

codecov · 2024-09-27T00:01:41Z

Codecov Report

Attention: Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.54%. Comparing base (5153307) to head (217bae0).
Report is 36 commits behind head on master.

Files with missing lines	Patch %	Lines
src/multithreading.jl	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1070      +/-   ##
==========================================
- Coverage   93.57%   93.54%   -0.04%     
==========================================
  Files          39       40       +1     
  Lines        6071     6086      +15     
==========================================
+ Hits         5681     5693      +12     
- Misses        390      393       +3

☔ 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.

termi-official

Is there a sane way to unit test this?

fredrikekre · 2024-11-08T07:29:46Z

Thoughts on the name? task_local is OK I guess, but then it sounds like it would return something like https://github.com/vchuravy/TaskLocalValues.jl, which it doesn't (but perhaps it should?). scratch_copy?

KnutAM · 2024-11-08T08:16:03Z

I think having task in the name makes sense. Some suggestions are
task_copy, task_scratch, or task_local_copy

KnutAM

Nice!

Took a bit deeper look, looks good. Docs/changelog etc of course needed as well, but still just WIP AFAIU.

Should it also be added to InterfaceValues?

KnutAM · 2024-11-08T08:22:15Z

+# TODO: For typical use the quadrature rule is read-only, but seems safer to copy anyway?
+#       And might even be beneficial with e.g. NUMA?


I agree, I think the cost of copying here would always be negligible, and there is a risk when using e.g. PointValues where the quadrature point is changed!

I would argue that task_local should always be safe and that the caller is responsible to not call task_local on immutable stuff, if he does not want the duplication.

IIRC, at least in the past, it was beneficial that the thread that used the data was the one allocating it so thats why I thought maybe it is better to duplicate this too.

I am not sure if that is also true for constant shared memory data. I suggest that we keep the rationale in a doc string here tho, so if we revisit this we have the exact arguments at hand (and also for users).

termi-official · 2024-11-08T11:23:37Z

+# TODO: For typical use the quadrature rule is read-only, but seems safer to copy anyway?
+#       And might even be beneficial with e.g. NUMA?


I would argue that task_local should always be safe and that the caller is responsible to not call task_local on immutable stuff, if he does not want the duplication.

termi-official · 2024-11-08T11:32:37Z

Some code paths are not covered yet (see CodeCov result).

KristofferC · 2024-11-08T18:17:02Z

FWIW, I am not 100% sold on this. What was the issue with the previous way?

Also, calling it task_local when it basically seems to create a (somewhat selective) copy feels strange to me? What's task local about that?

fredrikekre · 2024-11-11T14:08:23Z

What was the issue with the previous way?

The issue is basically that there isn't a previous way:

CellValues you can copy (so you don't have to thread the quadrature and interpolation all the way into the global assembly in order to call the CellValue constructor to obtain a copy)
CellCache is not a problem to call the constructor for each task because the input dh should be available
local matrix/vector is OK to recreate with zeros or whatever
To get a task local assembler you have to call start_assemble(K, f; fillzero = false)... what?

The point of this PR is to get something consistent I guess. I am not really fond of the name either, but since it is typically used when you set up a new task the name "looks good", i.e.

cv = task_local(cellvalues)
Threads.@spawn assemble!( cv...)

Would a name like scratch_copy or something be better?

termi-official · 2024-11-11T14:24:30Z

Would a name like scratch_copy or something be better?

To get the ball rolling on this, in Thunderbolt I have implemented this exact concept some time ago decided for the name duplicate_for_parallel.

KnutAM · 2024-11-11T15:15:02Z

copy_for_task?

IMO scratch isn't so nice since we also pass information (e.g. DofHandler in CellCache)

(But I'm also good withtask_local or task_local_copy)

fredrikekre · 2025-09-02T14:11:53Z

I still think something like this would be nice to have. Can we come up with a better name? I don't think copy is the right verb exactly...

KnutAM · 2025-09-22T05:43:15Z

I still think something like this would be nice to have. Can we come up with a better name? I don't think copy is the right verb exactly...

What about task_local_instance if we don't like xxx_copy and don't want just task_local?

KnutAM · 2026-04-11T17:34:16Z

Would be good to get this settled I think, and also related to the discussion in #1291.

I suggest we make a vote - add 👍 or 👎 for the suggestions you like or don't like below, and ❤️ for your favourite. If not wanting to have a function like this at all, add a 💀 on this comment :)

(Hope I didn't miss any suggestions, if so please add)

KnutAM · 2026-04-11T17:34:22Z

task_local

KnutAM · 2026-04-11T17:34:27Z

scratch_copy

KnutAM · 2026-04-11T17:34:32Z

task_copy

KnutAM · 2026-04-11T17:34:37Z

task_scratch

KnutAM · 2026-04-11T17:34:42Z

task_local_copy

KnutAM · 2026-04-11T17:34:46Z

duplicate_for_parallel

KnutAM · 2026-04-11T17:34:50Z

copy_for_task

KnutAM · 2026-04-11T17:34:54Z

task_local_instance

Base automatically changed from fe/ohmythreads to master September 26, 2024 09:54

fredrikekre force-pushed the fe/task_local branch from 9ca9e44 to 237c1cb Compare September 26, 2024 09:55

fredrikekre mentioned this pull request Sep 26, 2024

Make default assembler thread safer #1067

Open

termi-official reviewed Sep 26, 2024

View reviewed changes

Comment thread src/FEValues/FunctionValues.jl

fredrikekre added feature parallelization labels Sep 27, 2024

termi-official reviewed Nov 7, 2024

View reviewed changes

fredrikekre force-pushed the fe/task_local branch from 6c767fe to 840b1c9 Compare November 8, 2024 00:47

KnutAM reviewed Nov 8, 2024

View reviewed changes

termi-official reviewed Nov 8, 2024

View reviewed changes

fredrikekre force-pushed the fe/task_local branch from 5cbdba8 to b75e345 Compare November 8, 2024 16:31

Add task_local for duplicating scratch data for concurrent use

8fd03fb

fredrikekre force-pushed the fe/task_local branch from b75e345 to 8fd03fb Compare November 8, 2024 16:34

fredrikekre commented Nov 8, 2024

View reviewed changes

Comment thread src/Quadrature/quadrature.jl Outdated

Comment thread src/FEValues/FacetValues.jl Outdated

Apply suggestions from code review

217bae0

KnutAM mentioned this pull request Mar 5, 2026

Minimal GPU example which preserves the element routine #1291

Open

3 tasks

		# TODO: For typical use the quadrature rule is read-only, but seems safer to copy anyway?
		# And might even be beneficial with e.g. NUMA?

Conversation

fredrikekre commented Sep 26, 2024

Uh oh!

Uh oh!

KnutAM commented Sep 26, 2024

Uh oh!

codecov Bot commented Sep 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

termi-official left a comment

Choose a reason for hiding this comment

Uh oh!

fredrikekre commented Nov 8, 2024

Uh oh!

KnutAM commented Nov 8, 2024

Uh oh!

KnutAM left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KnutAM Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

termi-official Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

fredrikekre Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

termi-official Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

termi-official Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

termi-official Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

termi-official Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

KristofferC commented Nov 8, 2024

Uh oh!

Uh oh!

Uh oh!

fredrikekre commented Nov 11, 2024

Uh oh!

termi-official commented Nov 11, 2024

Uh oh!

KnutAM commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredrikekre commented Sep 2, 2025

Uh oh!

KnutAM commented Sep 22, 2025

Uh oh!

KnutAM commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KnutAM commented Apr 11, 2026

Uh oh!

KnutAM commented Apr 11, 2026

Uh oh!

KnutAM commented Apr 11, 2026

Uh oh!

KnutAM commented Apr 11, 2026

Uh oh!

KnutAM commented Apr 11, 2026

Uh oh!

KnutAM commented Apr 11, 2026

Uh oh!

KnutAM commented Apr 11, 2026

Uh oh!

codecov Bot commented Sep 27, 2024 •

edited

Loading

KnutAM commented Nov 11, 2024 •

edited

Loading

KnutAM commented Apr 11, 2026 •

edited

Loading