Skip to content

Add D implementation of monte carlo #162

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

Merged
merged 2 commits into from
Jun 29, 2018

Conversation

SirSireesh
Copy link
Contributor

Its a fairly straight forward implementation. Simply run with rdmd monte_carlo.d

@Gathros Gathros added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label Jun 29, 2018
@Butt4cak3 Butt4cak3 requested a review from zsparal June 29, 2018 10:19
import std.typecons : tuple;

auto rnd = Random(unpredictableSeed);
return generate(() => tuple!("x", "y")(rnd.uniform01, rnd.uniform01))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should make this part a little more explicit, something like:

    auto radius = 1.0;
    auto piCount = generate(() => tuple!("x", "y")(rnd.uniform01, rnd.uniform01))
        .take(n)
        .count!(a => inCircle(a.x, a.y, radius));
    return 4.0 * piCount / (n * radius * radius);

I definitely think radius should be a variable just or clarity's sake and with that change, jamming the return value into one expression seems a bit over the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do the return bit, but its worth noting that the Julia code is being refactored to remove the radius variable entirely. I think that would be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing radius also works but I'd still introduce the separate variable for piCount.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@zsparal zsparal merged commit 6fd1371 into algorithm-archivists:master Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants