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

Extension of PR#679 to stdlib_stats_distribution_exponential #717

Merged
merged 7 commits into from Jun 15, 2023

Conversation

HugoMVale
Copy link
Contributor

Please refer to #679 for an explanation about the issue to solve and approach used. As discussed there, I've extended the approach to the exponential module.

IMO, this module still has other opportunities for improvement. For instance, I don't understand why rvs_exp_array_ does not call the corresponding scalar procedure rvs_exp_, but instead repeats the code in rvs_exp_0_. I will have a better look at it and, eventually, suggest another PR.

Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

LGTM.
Are the "new" behaviours ("NAN" instead of "error stop") reflected in the specs?

@HugoMVale
Copy link
Contributor Author

Not yet, but I've realized that shortcoming. I started started fixing it for normal and I will do so for exponential.

@HugoMVale
Copy link
Contributor Author

HugoMVale commented Jun 14, 2023

For the sake of clarity, the question raised above by @jvdp1 has been addressed with the last batch of commits. From my perspective, this PR is ready for review.

Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @HugoMVale . I'll merge it.

@jvdp1 jvdp1 merged commit 9630cc5 into fortran-lang:master Jun 15, 2023
15 checks passed
@HugoMVale HugoMVale deleted the stats-distribution-exponential branch June 24, 2023 13:32
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.

None yet

2 participants