Fix a bug preventing icosahedral irreps from being cached.

[kalekundert]

I noticed that _build_ico_irrep() gets called every time the Icosahedral group is instantiated, even though the results are supposed to be cached by joblib. This adds about 20s to every script, which makes debugging pretty painful.

Looking into this, I found that the problem is that the dictionary returned by _build_ico_irreps() can't be pickled. Unfortunately, joblib just silently fails to cache anything in this case, leaving no outward indication that anything is wrong. It turns out that a warning for this exact case was recently added by joblib/joblib#1359, but hasn't been released yet.

The specific reason why the dictionary returned by _build_ico_irrep() can't be pickled is that the keys are group elements, each group element has a reference to the group it belongs to, and it doesn't make sense to pickle groups. On a conceptual level, doing so would lead to a weird situation where the unpickled group elements would have references to a group created by the unpickling machinery, not the group passed into _build_ico_irrep(). This could easily lead to a lot of subtle bugs. It's worth noting that some sort of global singleton pattern for groups could circumvent this problem, but that would be a pretty big change for a pretty niche issue. On a practical level, it's not even possible to pickle an icosahedral group object, because its _irreps attribute holds references to closures returned by functions like build_trivial_irrep() and build_trivial_character(), and closures can't be pickled. There are a couple ways to possibly work around this, but again they would be big changes for a niche issue.

The most straight-forward solution is just to cache all the information needed to recreate the group elements, but not the actual group element objects themselves. This ultimately ends up requiring _build_ico_irrep() to be split into 3 functions, which definitely isn't pretty, but at least it keeps all the extra complexity confined to one place. Here are the three new functions:

• _build_ico_irrep_unpicklable(): generate the dictionary in the same way as before.

• _build_ico_irrep_picklable(): replace the group element keys with (value, param) tuples, which can be pickled. This is the function that's cached by joblib.

• In _build_ico_irrep(): recreate the original keys using the provided group and the cached tuples.

Ultimately, no code outside of _build_ico_irrep() should be affected by this change.

[Gabri95]

Hi @kalekundert

Nice, thanks a lot for catching this problem and for the convenient fix!

I only cleaned a bit your solution by merging the three methods into only two.
Caching seems to work correctly for me now.

Thanks a lot again!
Gabriele