Output NaN for gradient for HDivTrace element

[chrisrichardson]

Change the output from TraceError to np.nan for gradient of HDivT element.
The reason for this change is that ffc may generate tables for MixedElement containing gradients, even if not needed. See FEniCS/dolfinx#684.

[dham]

I think this is probably OK. @thomasgibson is this likely to break anything?

[thomasgibson]

I totally fine with this, but I think this might break TSFC since it catches these exceptions. It shouldn't require too much modification to TSFC to adapt this and catch np.nan instead and handle it accordingly.

[dham]

Catching np.nan is not trivial because it's not an exception. @chrisrichardson can you please ensure this change isn't going to break TSFC?

[chrisrichardson]

@dham - Can I have write access to the tsfc repo then? Probably the easiest way to check is to run your travis with this FIAT branch.

[dham]

I've just invited you.

[dham]

Looking at the fails on TSFC, I am now not so convinced that this PR is a good idea. The current implementation means that a user who does the wrong thing gets an immediate exception. This change would mean that they will observe a NaN much later, which is hard to debug.

Surely if FFC is calling an illegal action and knows that it's OK, it should just trap the exception and deal with it accordingly.

[chrisrichardson]

I'm not convinced that outputting an Exception is the right response, either.
Really it should just raise here, but various imperfections in ffc/tsfc and MixedElement require returning something....

We'll try and fix on ffc side to avoid returning np.nan..

However, I think the fix on lind 204 is still needed.

[michalhabera]

@dham There already is np.nan as a returned value in some invalid cases - see docstring. Raising exception in some invalid cases is anyway weird.
However, as Chris said, we are trying to not even ask for invalid tabulation from FFCx, lets see.

[dham]

I agree. None of the fixes here is particularly clean and it would be better if we could just raise the exception here.

The case where we currently return a nan was a hack to accommodate ffc, because ffc does not pass in the entity to tabulate on but tsfc does. Now that apparently FFCx is passing in entities we need another fix, and we can hopefully get rid of returning nans in any case. (Indeed, it would be nice to get rid of the default value for entity).

[chrisrichardson]

@dham @thomasgibson - I have reverted most of this, except for one line which seems wrong. Calling with order > 0 results in a ValueError - too many items to unpack. I don't think this is intended...

[dham]

Hi Chris. I'm fine with this. In principle one should fix tabulating derivatives but it look non-trivial and I am not aware of any current use cases, so it's hard to justify spending time on it.