Enzyme type piracy

[Red-Portal]

Hi @yebai ,

Could you revert the recent two commits and repackage them into a PR so we can address the type piracy issue mentioned by @wsmoses ?

This needs to happen ASAP before any additional changes (like bug patches) need to be made to the main branch.

[gdalle]

I'm also curious about the apparent need (demonstrated in that same commit) to stop using DI bindings for some backends. If you have encountered any specific issues with DI, I'm happy to take a look!

[seabbs]

encountered any specific issues with DI

I assume/would hope issues for these should be on this repo or other TuringLang repos as a way of focussing discussion and motivating action

[yebai]

@Red-Portal, apologies for the direct push to main, I forgot to create a new branch for the work. I will try to take a look later this week.

[gdalle]

I assume/would hope issues for these should be on this repo or other TuringLang repos as a way of focussing discussion and motivating action

I have asked in JuliaDiff/DifferentiationInterface.jl#992 if there are any actionable requests

[yebai]

The changes have been moved to PR #249. This is still work in progress and is not yet ready for review.

In addition, Enzyme segfaulted without the Statistics rules, which is why I asked coding agents to implement them. Whether autograd rules in this context constitute type piracy is debatable, but at this point, it is probably best to avoid defining them in AdvancedVI.

[gdalle]

It is not debatable whether such rules constitute type piracy. According to https://docs.julialang.org/en/v1/manual/style-guide/#avoid-type-piracy:

"Type piracy" refers to the practice of extending or redefining methods in Base or other packages on types that you have not defined. In extreme cases, you can crash Julia (e.g. if your method extension or redefinition causes invalid input to be passed to a ccall). Type piracy can complicate reasoning about code, and may introduce incompatibilities that are hard to predict and diagnose.

The method pointed by Billy in the commit (now #249) is

AdvancedVI.jl/ext/AdvancedVIEnzymeExt.jl

Lines 36 to 41 in 5bc3162

	function EnzymeRules.augmented_primal(
	config::EnzymeRules.RevConfigWidth{1},
	::Enzyme.Const{typeof(Statistics.mean)},
	::Type{<:Union{Enzyme.Active,Enzyme.Duplicated,Enzyme.DuplicatedNoNeed}},
	x::Enzyme.Duplicated{<:AbstractVector{<:Real}},
	)

Since AdvancedVI

• doesn't own the function EnzymeRules.augmented_primal
• doesn't own any of the types given as arguments (which belong to EnzymeRules, Enzyme or Statistics)

this is a textbook example of piracy. Such rules, if they don't exist already, belong in Enzyme.

[penelopeysm]

Where did the commits on main go? Did someone force push to main?

[gdalle]

[yebai]

The commits are in the PR, together with all discussions. @penelopeysm

@gdalle, can you stop your non-productive engagement?

[yebai]

For posterity, see chalk-lab/Mooncake.jl#1146 (comment).