Skip to content

Conversation

vchuravy
Copy link

What feature report was addressed?

We are evaluating the use of PreallocationTools for Trixi, but need to ensure that adding it does not break Enzyme support.

Checklist

  • Appropriate tests were added
  • The new feature was added in a way that does not break public API
  • [] New documentation related to the new feature was added
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.

Additional context

Enzyme requires the shadow heap values to have the same type and shape as the primals. Therefore dual_du can actually be smaller than with ForwardDiff.

Supporting Enzyme batch mode is only possible on v1.11 or by allocating memory / doing unsafe operations.

My biggest worry would be user who attempt to use Enzyme over ForwardDiff. Then in the case of Const(stod) Enzyme and ForwardDiff would compete for the same memory.
The case of Duplicated(stod, Enzyme.make_zero(stod)) does the right thing in this instance.

@ChrisRackauckas
Copy link
Member

What feature report was addressed?

It wasn't, I couldn't figure out how to make it work. So for now we've required that if someone has preallocated caches, then they have to choose ForwardDiff or Enzyme and cannot support both. I asked @wsmoses if there was some way so that the extra caches from here somehow hook in, or worst case scenario if they are just ignored so the code is compatible, that is fine. But there has always been the issues with reinterpret and resize! in there, so it just never compiled.

But if you've got a way to get around this, that's great!

@vchuravy
Copy link
Author

But if you've got a way to get around this, that's great!

This PR makes at least example 1 from the readme work in forward mode. I believe the only true issue is Enzyme over ForwardDiff (and potentially Enzyme 2nd order Enzyme as well, so I should add a hessian test).

Do you have any more interesting corner cases, I could throw at this?

@ChrisRackauckas
Copy link
Member

I think the two cases are either putting the cache in p or enclosed in f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants