Make default assembler thread safer

[fredrikekre]

This patch make the assembler thread safe by using TaskLocalValue for the buffers. This simplifies writing multi-threaded assembly since there is no need to create multiple assemblers to put in scratch spaces.

[termi-official]

How much overhead does this cause for large problems?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.73%. Comparing base (c04c5ba) to head (a8bb38c).
Report is 7 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1067   +/-   ##
=======================================
  Coverage   93.72%   93.73%           
=======================================
  Files          39       39           
  Lines        6011     6015    +4     
=======================================
+ Hits         5634     5638    +4     
  Misses        377      377           

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

[fredrikekre]

assemble_global for the heat eq tutorial goes from 129μs to 139μs. A single call to assemble! goes from 99.107ns to 154.133ns. This should basically be the worst case because the element matrix is so small (4x4).

But I just realized that TaskLocalValue{Vector{T}} isn't concrete, so these times might be possible to reduce a bit.

Ok, 140μs and 123ns after making it concrete. Hmm.

[KnutAM]

On the one hand, its nice to make the assemblers thread safe. But is this, i.e. trying to make as much as possible thread safe, the way to go for facilitating multithreading? Couldn't one argue, from a user-perspective, that even CellValues should use the same constructs then? (but I don't think that makes sense performance-wise).

I think I'd rather want a clear interface wrt. making copies for multithreading (currently, copy works for AbstractValues, but that's not documented AFAIK (used in Helmholz gallery I think though). But perhaps defining copy for assemblers would make sense too? Or use a separate name for making copies for threading of Ferrite objects?

[fredrikekre]

I just find it a bit weird that you have to call start_assemble multiple times, for example, and make sure only one of the resets the matrix/vector to 0...

The assembler is a global object like the dofhandler/grid/stiffness matrix and doesn't feel as "scratchy" as cellvalues.

[KnutAM]

I just find it a bit weird that you have to call start_assemble multiple times, for example, and make sure only one of the resets the matrix/vector to 0...

Agreed - that's why I would like a copy_for_threading, but not sure how easy it is to make the interface sufficiently clear?

The assembler is a global object like the dofhandler/grid/stiffness matrix and doesn't feel as "scratchy" as cellvalues.

True, but e.g. a CellCache also contains global objects (unless #1049 is accepted), but would probably (but I don't know) be slow if made thread-safe?

And a side-note: Calling the assembler thread-safe could be dangerous, since coloring is still normally required?

[fredrikekre]

True, but e.g. a CellCache also contains global objects (unless #1049 is accepted), but would probably (but I don't know) be slow if made thread-safe?

I still think it is a bit different because we don't modify it

[KnutAM]

I still think it is a bit different because we don't modify it

You mean that we don't modify the global parts of the CellCache?

But I don't want to stop this efforts by being negative off course!
Perhaps I'm just too into my thinking for FerriteAssembly, where I think of assemblers as some kind of accumulator, where I find its reasonable that you need one "accumulator" per task, and then after looping these may (or may not) be "gathered" into the end result. (Of course, this is not fully consistent with assemblers, since they do the gathering every time by modifying the global entities, and just require coloring).

[KristofferC]

I do like that you don't need to duplicate the assembler in the scratch value as done now. Imo I think we should go with this assuming the overhead can be considered negligible for normal problems that are run without threading.

[fredrikekre]

Agreed - that's why I would like a copy_for_threading, but not sure how easy it is to make the interface sufficiently clear?

As an alternative, or maybe in addition to this PR, #1070 introduces something like this.