There are two types of memory sharing we need to consider:
aliasing (i.e. equivalence, arr1 === arr2)
reshaping of arrays whose eltype is a union of bits types
to_vec handles (1) and (2) correctly, but more work is necessary to make (2) work correctly in reverse-mode. The issue is that Base.deepcopy only pays attention to (1). Since we deepcopy arguments, keywords, callable, etc in multiple places, to_vec only sees arguments that alias in the sense of (1); those in the sense of (2) no longer share memory. This causes Enzyme and FiniteDifferences to disagree (in reality, it causes a dimension mismatch).
I see 2 ways to handle this:
deepcopy the arguments passed to Enzyme. I don't like this for several reasons. First, the user can then not manually check the state of the inputs they pass to a tester, which could be useful for mutating functions. Second, in principle a rule could depend on 2 arrays in the input sharing memory, and the testers wouldn't then work correctly for this rule.
Carefully check that we only deepcopy when needed. In reality we will probably need our own deepcopy implementation that uses our AliasDict instead of IdDict. Alternatively, we could recursively detect all the fields/entries that follow (2), deepcopy only one array that shares memory, prepopulate an IdDict, and pass to Base.deepcopy_internal. I suspect that's more work than just rolling our own deepcopy.
There are two types of memory sharing we need to consider:
arr1 === arr2)to_vechandles (1) and (2) correctly, but more work is necessary to make (2) work correctly in reverse-mode. The issue is thatBase.deepcopyonly pays attention to (1). Since we deepcopy arguments, keywords, callable, etc in multiple places,to_veconly sees arguments that alias in the sense of (1); those in the sense of (2) no longer share memory. This causes Enzyme and FiniteDifferences to disagree (in reality, it causes a dimension mismatch).I see 2 ways to handle this:
deepcopythe arguments passed to Enzyme. I don't like this for several reasons. First, the user can then not manually check the state of the inputs they pass to a tester, which could be useful for mutating functions. Second, in principle a rule could depend on 2 arrays in the input sharing memory, and the testers wouldn't then work correctly for this rule.deepcopyimplementation that uses ourAliasDictinstead ofIdDict. Alternatively, we could recursively detect all the fields/entries that follow (2), deepcopy only one array that shares memory, prepopulate anIdDict, and pass toBase.deepcopy_internal. I suspect that's more work than just rolling our owndeepcopy.