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

Adding support for FunctionChains.jl? #222

Closed
oschulz opened this issue Jun 22, 2022 · 9 comments
Closed

Adding support for FunctionChains.jl? #222

oschulz opened this issue Jun 22, 2022 · 9 comments

Comments

@oschulz
Copy link
Collaborator

oschulz commented Jun 22, 2022

In the spirit of #199, which resulted in InverseFunctions and ChangesOfVariables: I've been looking for a lightweight package that defines composed functions that go beyond Base.ComposedFunction, so that allow for arrays/iterators/tuples of functions, type-stable and performant. I couldn't really find anything lightweight that provides this (, though obviously the ML packages all have their own equivalent, and Bijectors has Composed.

Update: There's FunctionChains.jl now.

(@giordano, did I overlook something?)

@willtebbutt, @devmotion, @torfjelde, @cscherrer would there be interest in creating such a common lightweight package like that? I'd volunteer to draft it.

@cscherrer
Copy link

Sounds good! I've often hoped for a common interface for this sort of thing, especially the currently-incompatible diffeomorphism implementations that are spread across a few packages like this one.

One thing I think it's important to account for is that high-dimensional transforms are themselves often implemented in terms of iteration. So we need ways to compose these iterations "vertically" and "horizontally". I've mostly worked with TransformVariables; I'm not sure how similar or different Bijectors approach might be

@oschulz
Copy link
Collaborator Author

oschulz commented Jul 19, 2022

I've registered FunctionChains.jl. It should, in principle, be able to replace Bijectors.Composed, though the result will obviously not be a subtype of Bijector (is that important, though?).

@oschulz
Copy link
Collaborator Author

oschulz commented Jun 5, 2023

I could add support for FunctionChains to Bijectors, if there's interest, e.g. via bi-directioinal conversion in a package extension?

@oschulz oschulz changed the title Lightweight package for composed functions (function chains) Adding support for FunctionChains.jl? Jun 5, 2023
@devmotion
Copy link
Member

More a general question, not an actual objection at this point, but similar to the discussion in JuliaDiff/AbstractDifferentiation.jl#88 it's unclear to me if bi-directional conversions should be defined here or if only Base.convert(::Type{T}, ...) for types that Bijectors owns should be defined in a possible extension of it.

@oschulz
Copy link
Collaborator Author

oschulz commented Jun 5, 2023

bi-directional conversions should be defined here or if only Base.convert(::Type{T}, ...) for types that Bijectors owns should be defined in a possible extension of it.

I agree, it would be nice to have a kind of convention here, to avoid "extension piracy". On the other hand, maintenance wise it would be much easier (versioning, CI, etc.) to not split and extension that "bridges" two package in respect to a specific topic, but to host the complete extension in one of them. Ideally the package maintainers would come to some agreement where to put the extensions in cases where the choice is not obvious.

If the two packages is very lightweight/abstract, I think would would technically better to host the extension in the "heavy" package. That way, one can use a non-weak depencency instead of Requires on Julia <v1.9, which will improve load times there. In this specific case here, for example, FunctionChains can be very lightweight (I'll move most of it's dependencies into extensions, and Bijectors already shares pretty much all of them already anyway). So Bijectors depending on FunctionChains on Julia <= v1.8 and using and extension on >= v1.9 seems better, load-time wise, than FunctionChains using Requires on Julia <= v1.8 (since Bijectors is much heavier) and using an extension on >= v1.9.

@torfjelde
Copy link
Member

I could add support for FunctionChains to Bijectors, if there's interest, e.g. via bi-directioinal conversion in a package extension?

Shouldn't FunctionChains just "work" with Bijectors? Or am I missing something?

@oschulz
Copy link
Collaborator Author

oschulz commented Jun 5, 2023

I think you're right @torfjelde - I hadn't noticed that Bijectors.Composed <: Bijector has gone. So Bijectors wouldn't require FunctionChain to be a subtype of Bijector anymore and no conversion is necessary?

@torfjelde
Copy link
Member

Exactly:) Now it should "just work" 👍

@oschulz
Copy link
Collaborator Author

oschulz commented Jun 10, 2023

Thanks @torfjelde !

@oschulz oschulz closed this as completed Jun 10, 2023
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

4 participants