The weak key cache for ufl to FInAT conversion is too weak

[pbrubeck]

Suppose I'm building Q_k(hex) = Q_k(quad) x CG_k(interval) on an extruded mesh. Right now I see 2 calls to the FIAT constructor for CG, where I am only expecting a single one.

I suspect that the conversion takes in each factor in the tensor product from left to right. Q_k(quad) gets decomposed into CG_k(int) x CG_k(int) and the cache is used here, but the decomposition of the quad element is not referenced anywhere else in the hexahedral element, thus the cache is erased. For the next factor of the 3D element, CG_k(int), we have to call the constructor again.

I get the desired single call by doing _cache = {} in place of https://github.com/firedrakeproject/tsfc/blob/b9158b262db6324afda1adbaf7dee3ec637d2686/tsfc/finatinterface.py#L273

[wence-]

That sounds about right, but replacing this cache with one that is never evicted is also not good.

How about if the top-level call also takes a "temporary" dictionary cache in that it can populate as it goes which can serve as a "backup" lookup location. That way top-level elements are still cached "long term" but their construction can avoid this problem.

I suppose the reason this is a problem is that you're creating finat elements from short-lived UFL elements and so the top-level cache is also not that helpful? The original design was kind of predicated on the idea that you have long-lived functionspace objects that hold a UFL elements (which is therefore also long-lived) so that the cache here is useful.

[pbrubeck]

Wouldn't what you are suggesting only cover the case of reusing all 1D elements within a single function space? Ideally we would wish that on every function space (I'm thinking about a problem with variables in H(grad), H(curl), H(div), and L2), we are only constructing a single instance of each CGk and DG{k-1} 1D elements.

[wence-]

Probably yes. I think if you want to fix this without leaking space, then you want the top-level ufl element to hold on to its pieces while you construct the matching finat element.

Is making these elements a performance problem in an actual simulation? You would think that they are only made once, but maybe I am wrong.

[pbrubeck]

This is not affecting performance too much, I would just like to make the code neat. I have been constantly claiming that the FDM element is contructed from eigenfunctions that are computed only once on the reference interval.