WIP add task_local for duplication things for multithreading

[fredrikekre]

No description provided.

[KnutAM]

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

[codecov]

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

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.

[termi-official]

Is there a sane way to unit test this?

[fredrikekre]

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]

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]

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!

[termi-official]

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.

[fredrikekre]

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.

[termi-official]

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]

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]

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

[termi-official]

Still.

[KristofferC]

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]

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]

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]

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)