Added `output_length` and `output_size` #270

torfjelde · 2023-06-17T20:56:21Z

This is needed to ensure that transformed distributions are handled correctly now that transformations can change the support.

Related: #265

and sizes for transformations

src/transformed_distribution.jl

src/bijectors/corr.jl

test/bijectors/corr.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

src/bijectors/corr.jl

devmotion · 2023-06-18T06:59:27Z

src/bijectors/corr.jl

 end

+function output_size(::VecCorrBijector, sz::NTuple{2})
+    @assert sz[1] == sz[2]


Maybe make this a proper, more descriptive error?

src/bijectors/corr.jl

src/interface.jl

src/transformed_distribution.jl

Co-authored-by: David Widmann <[email protected]>

src/interface.jl

src/bijectors/corr.jl

torfjelde · 2023-06-18T09:45:24Z

Cheers @devmotion ! I did all your suggestions + removed output_length due to it's limited use. Also added more descriptive error 👍

torfjelde · 2023-06-18T09:47:25Z

Project.toml

 name = "Bijectors"
 uuid = "76274a88-744f-5084-9051-94815aaf08c4"
-version = "0.12.8"
+version = "0.13.0"


Just for the record: this isn't actually a breaking change, but I want to wait with a new release until we've merged this PR + #271 and #263 , i.e. we have have proper support for everything.

With the changes to Stacked, this is now indeed a breaking PR.

torfjelde · 2023-06-18T10:05:12Z

I'm fixing Stacked atm and I think that should probably go in this PR otherwise it'll buggy.

src/bijectors/stacked.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

test/bijectors/stacked.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

with tuple because recursive implementation fail

test/interface.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

src/bijectors/stacked.jl

src/transformed_distribution.jl

Co-authored-by: David Widmann <[email protected]>

torfjelde · 2023-06-18T16:17:19Z

Would be nice to get this merged somewhat quickly if there's nothing signficant holding it back, as everything related to "proper support for contrainsted distributions in Turing" is held back by this 😬

Note that the Stacked really needs a rework, as have been noted many times before, but I don't think we should do that now.

devmotion

Two minor comments, otherwise it looks good to me.

src/interface.jl

src/transformed_distribution.jl

Co-authored-by: David Widmann <[email protected]>

torfjelde · 2023-06-18T21:45:32Z

Awesome; thanks David! ❤️

torfjelde added 4 commits June 17, 2023 21:49

added output_length and output_size to compute output, well, leengths

6b46e34

and sizes for transformations

added tests for size of transformed dist using VcCorrBijector

41d0b06

use already constructed transfrormation

a76f18a

TransformedDistribution should now also have correct variate form

6afc77e

github-actions bot reviewed Jun 17, 2023

View reviewed changes

src/transformed_distribution.jl Outdated Show resolved Hide resolved

src/transformed_distribution.jl Outdated Show resolved Hide resolved

added proper variateform handling for VecCholeskyBijector too

a4c5683

github-actions bot reviewed Jun 17, 2023

View reviewed changes

src/bijectors/corr.jl Outdated Show resolved Hide resolved

test/bijectors/corr.jl Outdated Show resolved Hide resolved

torfjelde and others added 3 commits June 17, 2023 22:41

Apply suggestions from code review

ea724ee

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

added output_size impl for Reshape too

387ef5a

bump minor version

acb5e8f

torfjelde mentioned this pull request Jun 18, 2023

Vector-version for PDBijector #271

Merged

devmotion reviewed Jun 18, 2023

View reviewed changes

Apply suggestions from code review

3391735

Co-authored-by: David Widmann <[email protected]>

torfjelde commented Jun 18, 2023

View reviewed changes

src/interface.jl Outdated Show resolved Hide resolved

Update src/interface.jl

b524ebb

torfjelde commented Jun 18, 2023

View reviewed changes

src/bijectors/corr.jl Outdated Show resolved Hide resolved

Update src/bijectors/corr.jl

d6dc906

torfjelde commented Jun 18, 2023

View reviewed changes

torfjelde added 3 commits June 18, 2023 11:22

reverted removal of length as we'll need it now

280708b

updated Stacked to be compat with changing sizes

2069d69

forgot to commit deetion

f533a79

github-actions bot reviewed Jun 18, 2023

View reviewed changes

src/bijectors/stacked.jl Outdated Show resolved Hide resolved

src/bijectors/stacked.jl Outdated Show resolved Hide resolved

src/bijectors/stacked.jl Outdated Show resolved Hide resolved

torfjelde and others added 3 commits June 18, 2023 11:27

Apply suggestions from code review

56b8834

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

add testing of sizes to test_bijector

098a9c0

some more tests for stacked

4e14bb2

github-actions bot reviewed Jun 18, 2023

View reviewed changes

test/bijectors/stacked.jl Outdated Show resolved Hide resolved

torfjelde and others added 2 commits June 18, 2023 11:36

Update test/bijectors/stacked.jl

def7c6f

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

added awful generated function to determine output ranges for Stacked

fe36875

with tuple because recursive implementation fail

torfjelde added 3 commits June 18, 2023 12:31

added slightly more informative comment

bbfaf19

format

bf68124

more fixes to that damned Stacked

45a9850

github-actions bot reviewed Jun 18, 2023

View reviewed changes

test/interface.jl Outdated Show resolved Hide resolved

torfjelde and others added 3 commits June 18, 2023 13:06

Update test/interface.jl

1f0c374

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

specialized constructors for Stacked further

a917c2b

fixed bug in output_size for CorrVecBijector

cdd951a

devmotion reviewed Jun 18, 2023

View reviewed changes

src/bijectors/stacked.jl Outdated Show resolved Hide resolved

src/transformed_distribution.jl Show resolved Hide resolved

Apply suggestions from code review

5dbd829

Co-authored-by: David Widmann <[email protected]>

devmotion approved these changes Jun 18, 2023

View reviewed changes

src/interface.jl Outdated Show resolved Hide resolved

src/transformed_distribution.jl Outdated Show resolved Hide resolved

Apply suggestions from code review

04f6990

Co-authored-by: David Widmann <[email protected]>

torfjelde merged commit 2147089 into master Jun 18, 2023

delete-merged-branch bot deleted the torfjelde/size-interface branch June 18, 2023 22:08

sethaxen mentioned this pull request Jun 20, 2023

Add API function to retrieve size of bijector output from bijector input #265

Closed

Added output_length and output_size #270

Added output_length and output_size #270

Uh oh!

Conversation

torfjelde commented Jun 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devmotion Jun 18, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

torfjelde commented Jun 18, 2023

Uh oh!

torfjelde Jun 18, 2023

Choose a reason for hiding this comment

Uh oh!

torfjelde Jun 18, 2023

Choose a reason for hiding this comment

Uh oh!

torfjelde commented Jun 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

torfjelde commented Jun 18, 2023

Uh oh!

devmotion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

torfjelde commented Jun 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Added `output_length` and `output_size` #270

Added `output_length` and `output_size` #270

torfjelde commented Jun 17, 2023 •

edited

Loading