stop annotation inside PointEvaluator

[leo-collins]

Stops unnecessary taping inside PointEvaluator.evaluate

[connorjward]

Why is this unnecessary? Do we tape a higher level operation than this? Is it not implemented?

In assemble.py we don't tape things but we assert this by having a check: https://github.com/firedrakeproject/firedrake/blob/main/firedrake/assemble.py#L1026

I'm wondering if something like that would be more suitable here.

[leo-collins]

PointEvaluator.evaluate returns a numpy array and so doesn't work with the adjoint. If users want to do point evaluations with adjoint then they need to interpolate into a vom manually.

[connorjward]

PointEvaluator.evaluate returns a numpy array and so doesn't work with the adjoint. If users want to do point evaluations with adjoint then they need to interpolate into a vom manually.

In that case I would do the more aggressive thing and crash if taping is enabled. Note that this is fine if PointEvaluator.evaluate is used inside a taped operation because taping is always disabled inside taped operations.

[JHopeCollins]

In that case I would do the more aggressive thing and crash if taping is enabled. Note that this is fine if PointEvaluator.evaluate is used inside a taped operation because taping is always disabled inside taped operations.

I do not think that this is the right thing to do at all. There are plenty of operations that are not taped but we do not crash on if taping is activated. It is perfectly acceptable to do non-taped operations as long as they are not meant to be part of the functional calculation. This is what the Taylor test and the adj_value is None checks in pyadjoint are for.

If we think that non-taped operations should crash if taping is on then this is a big policy change and should be brought to the meeting for discussion.

[connorjward]

In that case I would do the more aggressive thing and crash if taping is enabled. Note that this is fine if PointEvaluator.evaluate is used inside a taped operation because taping is always disabled inside taped operations.

I do not think that this is the right thing to do at all. There are plenty of operations that are not taped but we do not crash on if taping is activated. It is perfectly acceptable to do non-taped operations as long as they are not meant to be part of the functional calculation. This is what the Taylor test and the adj_value is None checks in pyadjoint are for.

If we think that non-taped operations should crash if taping is on then this is a big policy change and should be brought to the meeting for discussion.

This is a fair point. We should decide what to do more universally. Adding it to assemble made some sense when we did it because the replacement is obvious but I can see that not all of our untaped API looks like that.

It could also be argued that taking a stronger approach will help people avoid taping bugs, but a warning may be sufficient. Happy to discuss.