Skip to content
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

Factor out backing and construct into a separate lightweight package? #668

Open
oschulz opened this issue Feb 28, 2024 · 6 comments
Open

Comments

@oschulz
Copy link
Contributor

oschulz commented Feb 28, 2024

We have duplicate implementations of what backing and construct do scattered all over the ecosystem, explicitly (e.g. ConstructionBase, Functors, ...) and implicitly in the internals of many packages.

The approach and implementation in ChainRulesCore is, to my knowledge, very solid and obviously extensively field-tested. Should we factor this out into a seperate package to make it more directly accessible? ChainRulesCore itself isn't that lightweight a dependency anymore, but a package with just backing and construct could be, I think.

@oxinabox
Copy link
Member

oxinabox commented Mar 1, 2024

I don't particularly see a pressing need to remove this duplication.
Further for construct it is not well tested as nothing uses our accumulation functionality AFAIK.
and for backing it's not really part of the public API.

If there was such a need, I would lean towards depending on ConstructionBase.

@oschulz
Copy link
Contributor Author

oschulz commented Mar 1, 2024

I don't particularly see a pressing need to remove this duplication.

With less duplication, more people might be encouraged to specialize these for their types (e.g. types with cache fields, etc.).

backing it's not really part of the public API

Oh, I didn't realize that. I have been using it in more complex pullbacks - what's the recommended way to get at the "contents" of a tangent?

@oxinabox
Copy link
Member

oxinabox commented Mar 5, 2024

what's the recommended way to get at the "contents" of a tangent?

There isn't one. I also have been using it in more complex tangents.
Possibly we should decide it is part of the public API, and write guidelines for it's use etc.
In theory one isn't required to ever use it, since the getproperty and iterate overloads should be enough.
When are you finding problems with it?
(The problems i find are mostly to do with type-stability)

@oschulz
Copy link
Contributor Author

oschulz commented Mar 5, 2024

In theory one isn't required to ever use it, since the getproperty and iterate overloads should be enough.

I've been using it a few times where I need to deal with or forward the "contents" of a tangent in a generic fashion, like here https://github.com/oschulz/ForwardDiffPullbacks.jl/blob/2ef833bc63886a4be4904e18f49066847c022edd/src/fwd_back.jl#L42 and here https://github.com/oschulz/ValueShapes.jl/blob/ca3b61eabec20445ea0798df3d50a148584e216e/src/named_tuple_shape.jl#L449 and in some testing code.

I guess custom code for doing math with tangents will be hard(er) to write without it, too. In general it makes sense to me to have a function to get the "undecorated" contents of tangents.

@oxinabox
Copy link
Member

oxinabox commented Mar 5, 2024

For that use case you can use NamedTuple(tangent), I hope.

@oschulz
Copy link
Contributor Author

oschulz commented Mar 5, 2024

For that use case you can use NamedTuple(tangent), I hope.

Thanks, I'll give that a try.

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

No branches or pull requests

2 participants