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

Probability Distribution and Statistical Functions -- PRNG Module #271

Merged
merged 78 commits into from
Feb 6, 2021

Conversation

Jim-215-Fisher
Copy link
Contributor

@Jim-215-Fisher Jim-215-Fisher commented Dec 19, 2020

This is the second round of probability distribution and statistical functions, continuing from #240. Since the whole modular structure has been changed, each distribution will have its own PR.
This PR contains random number generators used for probability distributions. Files uploaded are:
src/stdlib_stats_distribtuion.PRNG.fypp
src/CmakeLists.txt
src/Makefile.manual
src/tests/test_distribution_PRNG.f90
src/tests/CmakeLists.txt
src/tests/Makefile.manual
doc/specs/stdlib_stats_distribution_PRNG.md
doc/specs/index.md

@Jim-215-Fisher Jim-215-Fisher changed the title Distribution prng Probability Distribution and Statistical Functions -- PRNG Module Dec 20, 2020
@jvdp1
Copy link
Member

jvdp1 commented Dec 20, 2020

Thank you @Jim-215-Fisher for this PR. Is it ready for review?

@Jim-215-Fisher
Copy link
Contributor Author

Thank you @Jim-215-Fisher for this PR. Is it ready for review?

Yes. I think it is ready.

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.

Thanks @Jim-215-Fisher for this PR. Here are some first comments.

@jvdp1
Copy link
Member

jvdp1 commented Jan 18, 2021

Thank you @Jim-215-Fisher for this PR. I have no additional comments for now.
@ivan-pi @milancurcic any other comments? If not, I suggest that we mege this PR as is. So we can move to the next one associated to this PR. Are you ok with that?

@ivan-pi
Copy link
Member

ivan-pi commented Jan 18, 2021

I agree. +1 to merge and move to the next pull request.

Copy link
Member

@ivan-pi ivan-pi left a comment

Choose a reason for hiding this comment

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

I would recommend squashing commits before merging.

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.

@milancurcic Can you merge it if you are ok with it,please?

@milancurcic
Copy link
Member

Thank you all, I will review this weekend and merge.

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

Thank you @Jim-215-Fisher and reviewers. I left two more suggestions, feel free to apply them or not.

Comment on lines 115 to 117
data int01, int02, int03/-7046029254386353131_int64, &
-4658895280553007687_int64, &
-7723592293110705685_int64/
Copy link
Member

Choose a reason for hiding this comment

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

These could be rewritten as integer(int64), parameter, which IMO would be cleaner, and I think just as efficient as data statement.

@ivan-pi
Copy link
Member

ivan-pi commented Feb 5, 2021

Thank you all, I will review this weekend and merge.

I would recommend squashing commits, not to polute the history with the numerous commits.

@milancurcic
Copy link
Member

@Jim-215-Fisher Please let us know if this is good to go, and we'll squash and merge.

@Jim-215-Fisher
Copy link
Contributor Author

@Jim-215-Fisher Please let us know if this is good to go, and we'll squash and merge.

OK. It is ready to go.

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.

4 participants