-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Implement named (item) closures #852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I initially figured this wasn't a very necessary feature, but it is almost impossible to construct recursive closures using the current system. That seems a good motivation for item closures. |
I am of two minds. It is true that recursive closures are almost impossible, which is a shame. On the other hand, getting the semantics of when the capture code occurs is a pain. In practice, if I want recursive closures, then I just create a record for the data I need and make an impl for it. Not sure if this is an acceptable workaround. |
but I'd be inclined to close this bug for now. I don't feel like it's a burning problem. |
heh, I assigned this to myself? If it's not wanted all that badly, I'm happy to not do it, though ;-) |
* Add MLIR build infra * With tests * Update readme * Add initial infra * Continue pushing towards forward mode infra * Most primitive functioning test * Fix cmake * Handle control flow (fwd) * Add scf.for * Update enzyme-mlir.yml * Introduce a MLIR OpInterface and redistribute code Note that the interface is not yet implement by the ops. This will be done separately. * Fix CI * Fix CI * Fix format * Add python3-dev * Reduce link * Fix build and test Real includes must precede .inc files. A function was not returning anything leading to a segfault. * Factor gradient implementations out into external interface models MLIR-code must use interfaces instead of giant switches. External models allow for interfaces to be attached to foreign dialect ops without modifying them. * Factor out the type interface for shadow and null Introduce a separate type interface for types that will be handled by AD. This interface currently allows for defining the (unique) shadow type of the given type and for building a null value of the type. This is a step towards removing the dependency on the arithmetic dialect from the AD pass. * Drop AD pass dependency on the Arith dialect Now that null constant manipulation has been factored out into the interface, the only remaining use is an attempt to match a constant. Use the robust dialect-agnostic matcher instead. * Factor out common OpBuilder manipulation into the driver * Drop the dependency on FuncOps from GradientUtils We don't need to know the specific op class of a function-like op to clone it. This makes the autodiff interface independent of the func dialect, but the AD pass still depends on it because it creates a function call. There is currently no way of creating a function call in an abstracted way, short of which the pass dependency on the func dialect cannot be removed. * Update enzyme-mlir.yml * Only release llvm * Fix CI * Rebase * Simplify TA Co-authored-by: Alex Zinenko <zinenko@google.com>
We should probably be able to have named functions that can close over their environments.
I think we definitely want named copying closures. There shouldn't be anything fundamentally difficult about this, although these are items that behave differently that other items, and thus will be a bit annoying.
Named aliasing closures are trickier, since their upvars can get deinitialized. Probably the right solution, if we want them, is to check that their upvars are initialized when the name is referenced. (Graydon's proposal suggested doing the capture when the name is referenced for both aliasing and copying closures; I think that doing it for copying closures is extremely counterintuitive since it would be behaving substantially different from assigning it to a variable manually.)
Making the items be able to take type parameters might also be a bit fiddly, as the function will need to pull type parameters from both the closure and the arguments.
The text was updated successfully, but these errors were encountered: