Skip to content

Conversation

@leios
Copy link
Member

@leios leios commented Jun 29, 2018

Hey guys, a lot of people were mentioning that the julia code for monte carlo was unclear. After looking at it again, I agree. I really wanted to have some parameter radius in the function, just because I wanted to make it clear that the circle could be an arbitrary size. That said, @jiegillet brought it to my attention, that we clearly say we are using a unit circle in the code, and if we read in a radius of greater than the box length, the code is wrong.

For this purpose, I removed the radius parameter from the julia code (except in the in_circle() function, but I can remove it there too). I was just being overly cautious to try to make the text easier to read and understand and in the process actually made it more difficult for new contributors.

Let me know what you think. I should probably just remove the radius from the in_circle() function too, now that I think about it.

We need to put code in that makes sense, so whatever you guys think is best should be what we go with here.

@jiegillet
Copy link
Member

That's an improvement. Leaving radius in in_circle wouldn't bother me at all, but either is ok.


# Setting radius to 1 for unit circle
radius = 1
if (x_pos^2 + y_pos^2 < radius^2)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be return x_pos^2 + y_pos^2 < radius^2 in my opinion. I don't like unnecessary if/else blocks

@leios leios merged commit 0e2dbc5 into algorithm-archivists:master Jun 29, 2018
@zsparal
Copy link
Contributor

zsparal commented Jun 29, 2018

The overeager @leios merge strikes again 😃

@leios
Copy link
Member Author

leios commented Jun 29, 2018

Oh, shoot

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.

3 participants