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

Rework truncation based on log q #338

Merged
merged 12 commits into from
Nov 22, 2023
Merged

Rework truncation based on log q #338

merged 12 commits into from
Nov 22, 2023

Conversation

mj-will
Copy link
Owner

@mj-will mj-will commented Oct 24, 2023

Rework how the truncation based of the log-proposal probability (log q) to use the minium value of the training data.

Motivation

This could be an alternative to truncating the latent distribution. It should be less prone to over-constraining but may result in inefficient rejection sampling.

Changes

  • Add methods for sampling from the latent distribution to FlowModel and BaseFlow
  • Refactor FlowProposal.radius
  • Add new keyword arguments to FlowProposal.backwards_pass
  • Rename worst_q to min_log_q in FlowProposal.rejection_sample

Breaking changes

These changes are all to options that were not used by default and we generally recommended avoiding in the past, so I don't think they should impact users.

  • This PR removes the current method for truncation that uses the log-probability of the worst point
  • This PR removes the truncate keyword argument from FlowProposal
  • This PR renames the worst_q keyword argument to min_log_q in FlowProposal.rejection_sample

To-Do

  • Document the method
  • Fix existing unit tests
  • Add new unit tests
  • Add a simple integration test

@mj-will mj-will added enhancement New feature or request breaking changes Breaking API change labels Oct 24, 2023
@mj-will mj-will added this to the v0.10.0 milestone Oct 24, 2023
@mj-will mj-will modified the milestones: v0.10.0, Future Oct 31, 2023
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d5cd2b7) 93.26% compared to head (a1dcbb2) 93.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
+ Coverage   93.26%   93.29%   +0.02%     
==========================================
  Files          67       67              
  Lines        6300     6322      +22     
==========================================
+ Hits         5876     5898      +22     
  Misses        424      424              
Flag Coverage Δ
unittests 93.29% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@mj-will mj-will marked this pull request as ready for review November 22, 2023 15:58
@mj-will
Copy link
Owner Author

mj-will commented Nov 22, 2023

I'm deferring documenting this to a later PR since this would require a lot of additional documentation and the method is still not recommended.

@mj-will mj-will merged commit ea59056 into main Nov 22, 2023
23 checks passed
@mj-will mj-will deleted the truncate-log-q branch November 22, 2023 15:59
@mj-will mj-will modified the milestones: Future, v0.11.0 Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Breaking API change enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant