search

TuringLang / Bijectors.jl

Implementation of normalising flows and constrained random variable transformations
https://turinglang.org/Bijectors.jl/
MIT License
199 stars 32 forks source link

Rename `stack` to `stack_transforms` #256

Closed torfjelde closed 1 year ago

torfjelde commented 1 year ago

On Julia 1.9 we have Base.stack, which means that our exported Bijectors.stack has a name-clash, and overloading it in a useful way is too dangerous.

So I've renamed it. I also don't think this is being used anywhere but internally.

torfjelde commented 1 year ago

Couldn't we just extend Base.stack?

I tried, but because we are no longer forcing everything to be a subtype of Transform/Bijector, we can't extend it in a useful manner without very dangerous type-piracy.

devmotion commented 1 year ago

Ah, I see :+1: Then another question is, what is the advantage of having a separate stack/stack_transforms function over an additional constructor Stacked(bs...) = Stacked(bs)?

torfjelde commented 1 year ago

Ah, I see +1 Then another question is, what is the advantage of having a separate stack/stack_transforms function over an additional constructor Stacked(bs...) = Stacked(bs)?

I think the original idea was that it could specialize, e.g. stack(::Logit{0}...) could be implemented to result in Logit{1}, but now that we don't have these anymore, it's less clear what the advantage is.

torfjelde commented 1 year ago

This should be ready to go now, once the tests pass :+1:

torfjelde commented 1 year ago

@devmotion You good with this? Would be nice to get this merged so tests will pass again and we merge other PRs that are in the pipeline.