-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add renormalized laplace and gaussian distribution and kernels #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow thanks for putting this together in this short amount of time! This looks great to me overall -- I just left some inline comments, most of which are just cosmetics stuff. Feel free to merge this whenever you're ready.
src/b3d/modeling_utils.py
Outdated
p_below_low = tfp.distributions.Laplace(loc, scale).cdf(low) | ||
p_below_high = tfp.distributions.Laplace(loc, scale).cdf(high) | ||
log_integral_of_laplace_pdf_over_this_range = jnp.log( | ||
p_below_high - p_below_low | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For numerical stability, perhaps we can consider using Laplace.log_cdf
and Laplace. log_survival_function
?
(btw also just for clarity: I think log_integral_of_laplace_pdf_over_this_range
is actually referring to the integral of laplace within the range?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@horizon-blue can you please suggest the right code snippet we need to do log_cdf
and log_survival_function
here?
def sample(self, key, latent_color, color_scale, *args, **kwargs): | ||
return jax.vmap( | ||
genjax.truncated_normal.sample, in_axes=(0, 0, None, None, None) | ||
)(split(key, latent_color.shape[0]), latent_color, color_scale, 0.0, 1.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to you feel about usingCOLOR_MIN_VAL
and COLOR_MAX_VAL
instead of hard-coding the magic constant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!
@horizon-blue I made the suggested changes that I knew how to make! |
No description provided.