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

ExponentialParticleDistribution: Use rate-based formula #271

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

leroyvn
Copy link
Member

@leroyvn leroyvn commented Oct 4, 2022

Description

This PR changes the exponential particle distribution function to a rate-based formula. The original formula

$f : x \mapsto \dfrac{1}{\beta} \exp \left( - x / \beta \right)$

does not feel natural to all users (including myself), who may prefer the rate-based formula

$f : x \mapsto \dfrac{\lambda e^{-\lambda x}}{1 - e^{-\lambda}}$.

This PR still keeps the option of initialising this function with a scale parameter.

Checklist

  • The code follows the relevant coding guidelines
  • The code generates no new warnings
  • The code is appropriately documented
  • The code is tested to prove its function
  • The feature branch is rebased on the current state of the main branch
  • I updated the change log if relevant
  • I give permission that the Eradiate project may redistribute my contributions under the terms of its license

Copy link
Contributor

@nollety nollety left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks perfectly fine to me, just a few comments 😃
Since the formula was not incorrect per say, I'd change the PR title to something like:
ExponentialParticleDistribution: change formulation to rate-based

CHANGELOG.md Outdated Show resolved Hide resolved
@leroyvn leroyvn changed the title ExponentialParticleDistribution: Fix incorrect formula ExponentialParticleDistribution: Use rate-based formula Oct 4, 2022
@leroyvn leroyvn merged commit f5d4136 into main Oct 4, 2022
@leroyvn leroyvn deleted the fix_exponential branch October 4, 2022 14:20
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