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

Fix incorrect output dimensions in stacked bijectors #2066

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

bgroenks96
Copy link
Contributor

Fixes #2065 (still needs testing)

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4affc28) 0.00% compared to head (55fb3b5) 0.00%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #2066   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          22      22           
  Lines        1488    1492    +4     
======================================
- Misses       1488    1492    +4     
Files Changed Coverage Δ
src/variational/advi.jl 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yebai
Copy link
Member

yebai commented Aug 24, 2023

It looks good to me. Maybe add the MWE to tests?

@bgroenks96
Copy link
Contributor Author

Ok, is the revision in Project.toml automatically updated?

@yebai
Copy link
Member

yebai commented Aug 24, 2023

I don't think so. You need to bump it manually, too.

@bgroenks96
Copy link
Contributor Author

Ok so turns out that wasn't the fix, I had to add dispatches for output_length and transform. Please take a look and then run the tests again when you're ready.

@bgroenks96
Copy link
Contributor Author

I am also running the tests locally so we'll see if these changes break anything. I tried to be specific with the dispatches.

@coveralls
Copy link

coveralls commented Aug 24, 2023

Pull Request Test Coverage Report for Build 5964052388

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 0.0%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/variational/advi.jl 0 4 0.0%
Files with Coverage Reduction New Missed Lines %
src/variational/advi.jl 1 0.0%
Totals Coverage Status
Change from base Build 5884154416: 0.0%
Covered Lines: 0
Relevant Lines: 1492

💛 - Coveralls

@bgroenks96
Copy link
Contributor Author

I don't think this test failure is related to this PR...? but I am not sure from first glance.

@bgroenks96 bgroenks96 changed the title Fix incorrect output dimensions in multivariate bijectors Fix incorrect output dimensions in stacked bijectors Aug 25, 2023
@yebai
Copy link
Member

yebai commented Aug 25, 2023

I don't think this test failure is related to this PR...? but I am not sure from first glance.

Agreed that it seems not related to this PR

@bgroenks96
Copy link
Contributor Author

FWIW the tests passed locally for me.

Is anything else needed before merging?

@yebai yebai merged commit f891aa8 into TuringLang:master Aug 28, 2023
@yebai
Copy link
Member

yebai commented Aug 28, 2023

Thanks, @bgroenks96 -- a random CI failure went away after a second CI run.

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

Successfully merging this pull request may close these issues.

Incorrect bijector dimensions for models using SimplexBijector
3 participants