-
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
Modified truncate_normal with jax.random.truncated_normal #175
Conversation
Pull Request Test Coverage Report for Build 9993829303Details
💛 - Coveralls |
jax-ml/jax#10951 But I still think this should be better than using the previous clipping method. The comparison when simulating a truncated normal distribution with mean=0, std=1, vmin=-2, vmax=2 is shown below. |
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.
Thanks! The PR looks good to me
We should revert this PR because NEST also uses the previous truncate_normal in quanta generation. Here NEST defines two types of truncate_normal, one is the non-continuous version (termed |
…)" This reverts commit a74b86f.
Previously the truncate_normal distribution was sampled from a normal distribution and then clipping the result. This does not yield a continuous distribution. A more reasonable distribution should be like: https://en.wikipedia.org/wiki/Truncated_normal_distribution
This should affect the microphysics simulation. Maybe the previous version is intentional, so I will let experts comment on which behavior is more correct.