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

simplifying Julia code for Monte Carlo #178

Merged
merged 1 commit into from
Jun 30, 2018

Conversation

leios
Copy link
Member

@leios leios commented Jun 29, 2018

Hey guys, I screwed up. Sorry. Resubmitting a prematurely merged and then reverted PR... I think I learned my lesson, though!

Copy-paste from #159: 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.

Note: I updated after @Gustorn 's request.

@leios leios changed the title fixing return statment in in_circle() function simplifying Julia code for Monte Carlo Jun 29, 2018
@jiegillet jiegillet added the Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) label Jun 29, 2018
@jiegillet jiegillet requested a review from zsparal June 29, 2018 23:00
@zsparal zsparal merged commit 3e2097b into algorithm-archivists:master Jun 30, 2018
@leios leios deleted the monte_carlo_updates branch September 21, 2018 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants