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

Refactor cvi projection marginal rule (with proposal distribution) #430

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Nimrais
Copy link
Member

@Nimrais Nimrais commented Nov 15, 2024

Conjugate Variational Inference Enhancement PR

Overview

This PR enhances the Conjugate Variational Inference (CVI) projection method by adding proposal distribution support and flexible sampling strategies. These improvements address practical limitations in the current implementation, specifically for deterministic nodes with multiple interfaces.

Key Features

1. Proposal Distribution Support

  • Added ProposalDistributionContainer to maintain and update proposal distributions across iterations
  • Improved convergence by recycling posterior approximations as proposals for subsequent iterations
  • Resolved the issue where sampling directly from messages isn't always possible

2. Flexible Sampling Strategies

  • Implemented CVISamplingStrategy enum with two options:
    • FullSampling: Traditional approach with multiple samples (better accuracy)
    • MeanBased: Uses only distribution means (significantly faster)
  • Added helper functions to generate samples based on chosen strategy

Benefits

  • Performance: Mean-based sampling provides substantial speed improvements (benchmarks show ~50-70% faster execution)
  • Convergence: Improved approximation quality over multiple iterations with recycled proposals
  • Flexibility: Works better with deterministic nodes with many interfaces
  • Stability: More robust in complex models where direct sampling is problematic

Testing

The PR includes some tests covering:

  • Convergence quality analysis across multiple iterations
  • Performance benchmarking comparing sampling strategies
  • Correctness verification for different sampling approaches
  • Integration tests with typical use cases

@Nimrais Nimrais marked this pull request as draft November 15, 2024 14:18
@Nimrais Nimrais changed the title Refactor cvi projection marginal rule use proposal distribution Refactor cvi projection marginal rule (use proposal distribution) Nov 15, 2024
@Nimrais Nimrais changed the title Refactor cvi projection marginal rule (use proposal distribution) Refactor cvi projection marginal rule (with proposal distribution) Nov 15, 2024
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.75%. Comparing base (b88de4d) to head (d099a27).
Report is 38 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #430      +/-   ##
==========================================
+ Coverage   74.42%   74.75%   +0.33%     
==========================================
  Files         195      198       +3     
  Lines        5623     5721      +98     
==========================================
+ Hits         4185     4277      +92     
- Misses       1438     1444       +6     

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

@bvdmitri
Copy link
Member

@Nimrais ping

@bvdmitri
Copy link
Member

@Nimrais do we want to merge this?

@Nimrais
Copy link
Member Author

Nimrais commented Feb 26, 2025

@bvdmitri This PR works in principle. I have an example (in a model notebook) that this PR allows to run. I also added tests that show the convergence with the marginal approach. However, I am currently refactoring the ExponentialFamilyManifolds to address one of the reviews.

If you could review it and suggest the minimal possible changes needed before merging, it would be nice, as I am using this branch locally because CVIProjection works better in this branch for deterministic nodes with many interfaces.

More importantly, this PR solves an important problem: it's not always possible to sample from the messages; however, in some sense, it's always possible from marginals.

@Nimrais Nimrais marked this pull request as ready for review February 26, 2025 13:50
@Nimrais Nimrais requested a review from bvdmitri February 26, 2025 13:50
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.

2 participants