Make VI work on v4 #4582
Merged
Make VI work on v4 #4582
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ferrine I understand this is still WIP ? After #4579 (the rebase) replaces |
brandonwillard
force-pushed
the
v4
branch
2 times, most recently
from
93a096d to
c9fa127
Compare
twiecki
reviewed
May 4, 2021
ferrine
commented
Jun 18, 2021
|
@ferrine this was one of the last remaining PRs that still had |
|
I included the VI tests as a standalone job into the pytest workflow, so it's output is not contaminated by flaky tests that still exist on Running locally on Windows: List of test failuresHope this helps :) |
|
@brandonwillard I have a subset of functionality working. AEVB inference does not work and I start thinking maybe I mark this as NotImplemented? |
|
I agree that we can skip that.
…On Wed, Jul 28, 2021 at 4:47 PM Maxim Kochurov ***@***.***> wrote:
@brandonwillard <https://github.com/brandonwillard> I have a subset of
functionality working. AEVB inference does not work and I start thinking
maybe I mark this as NotImplemented?
if aevb: raise NotImplementedError("AEVB is not yet supported, open an issue once you need it https://github.com/pymc-devs/pymc3/issues")
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4582 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFETGDXT7YGZKQLTFRWAITT2AJ75ANCNFSM4Z7ATPNQ>
.
|
|
Hmm, the issue still appears with grouped inference that might be some shape bug/typo somewhere. I'll keep working on this. This should not be that complicated I hope |
Codecov Report
@@ Coverage Diff @@
## main #4582 +/- ##
==========================================
+ Coverage 81.44% 87.21% +5.76%
==========================================
Files 81 81
Lines 14204 14265 +61
==========================================
+ Hits 11569 12441 +872
+ Misses 2635 1824 -811
|
|
/pre-commit-run |
|
@ferrine I rebased this. |
|
One weird test is left and it's done. Also I remember to change skip to xfail for one test |
|
Tests are passing 🥳! You need to rebase from the rename, however. |
ricardoV94
reviewed
Feb 20, 2022
ricardoV94
reviewed
Feb 20, 2022
ricardoV94
reviewed
Feb 20, 2022
ricardoV94
reviewed
Feb 20, 2022
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
|
@ferrine I pushed some changes and closed outdated conversations. Can you go through the remaining open conversations and address/close them? Otherwise I think this is pretty ready to merge, and we can focus on refactoring/deprecating the remaining methods in follow up PRs |
ricardoV94
approved these changes
Feb 23, 2022
ricardoV94
reviewed
Feb 23, 2022
|
@ricardoV94 are we good to merge? |
|
Only one way to find out... |
|
I commented one instance where detailed type info from the docstring could be annotated in the signature. |
Co-authored-by: Michael Osthege <michael.osthege@outlook.com>
|
/pre-commit run |
|
/pre-commit-run |
|
No it's something that should be fixed on main, I merged a PR too wrongly. |
|
I was about to hit approve when I noticed this. Looks like the linux permissions on that file changed?? Could explain why it's not showing up in the diff view. Otherwise I'd say this is ready to be squash-merged |
Yeah, file permissions were weird, I fixed this |
|
Incredible -- congrats @ferrine and @ricardoV94! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VI in V4
Referencing #4523 Here to work on VI module
Step 1
The first goal of this PR is to fix test errors in
because there are a lot of errors
Step 2
TBD