Skip to content

Added intergration of Monte Carlo for PHP #431

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 10 commits into from
Oct 7, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions contents/monte_carlo_integration/code/php/monte_carlo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
declare(strict_types=1);

function in_circle(float $positionX, float $positionY, float $radius = 1): bool
{
return pow($positionX, 2) + pow($positionY, 2) < pow(abs($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.

I think you should just do
$positionX * $positionX + $positionY * $positionY + $positionZ * $positionZ
The abs also seems completley unnecessary since x^2 is always positive.

Copy link
Contributor

Choose a reason for hiding this comment

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

This still hasn't been resolved btw.

}

function random_zero_to_one(): float
{
return mt_rand(0, 100) / 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for the 100? The recomended way to get a random float is the same as C:
return mt_rand() / mt_getrandmax();

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, there is. We want number in interval <0,1> so I'm getting int in range <0,100> and dividing 100 to get required number

Copy link
Contributor

Choose a reason for hiding this comment

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

That results in only 101 posible values 0.000, 0.001, 0.002, ..., 0.999, 1.0, running over 101^2 times would then be meaningless, instead you should take advantage of the full range of floating point precision by dividing by mt_getrandmax()

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, it's changed, thanks for explanation :)

}

function monte_carlo(int $samples, float $radius = 1): float
{
$in_circle_count = 0;

for ($i = 0; $i < $samples; $i++) {
if (in_circle(random_zero_to_one(), random_zero_to_one(), $radius))
Copy link
Contributor

Choose a reason for hiding this comment

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

x and y position need to be multiplied by $radius for proper range of random values.

Copy link
Member Author

Choose a reason for hiding this comment

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

Haven't seen any of the other implementations use radius to multiply X or Y. So what's different in this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Other implementations chose for radius to be a constant 1.0, you put radius as a function parameter which default value 1.0
Either you can make radius constant 1.0 or implement it so it correctly calculates pi when the radius parameter is not equal to 1.0
Take a look at the python implementation, they generate random numbers from 0 to radius. The easiest way to do this is just multiply your existing random function by $radius.

$in_circle_count++;
}

return 4 * $in_circle_count / $samples;
}

$piEstimate = monte_carlo(100000000);
$percentError = abs($piEstimate - pi()) / pi() * 100;
echo sprintf('The estimate of PI is: %s', $piEstimate);
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. PHP has a printf function
  2. Probably should use %f for these values, php converts em anyway but no harm in being more correct.
  3. Why are we using PHP_EOL rather than \n, it doesn't work for single quote strings but they work fine in double quotes
    printf("The estimate of PI is: %f\n", $piEstimate);

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. it has, but there is not much difference and sprintf gives easier way to extract variable
  2. %f would generate X.XXXXX format when %s givex X.XX (depending on number of numbers after dot)
  3. PHP_EOL is constant depending on environment. from SO "PHP_EOL represents the endline character for the current system", so it's much prefered to use it

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough, I don't understand what you mean by "sprintf gives easier way to extract variable" but your other arguments make sense, I was under the impression PHP would \n to the correct line ending like C/C++.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's basically from my experience, where you don't print to console, but you need to work with strings, build them, etc. You can store sprintf to variable and work with it later. printf would print to output.

yes, it would but with PHP_EOL. otherwise if you specify \n it uses that, which might be problem on some platforms.

Copy link
Contributor

@Butt4cak3 Butt4cak3 Oct 4, 2018

Choose a reason for hiding this comment

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

You can store sprintf to variable and work with it later.

You do not do that, though, so there's no reason to use it. I have to agree with @Gorzoid here. printf() makes a little more sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

I give you this one, I'm gonna change it. But still I'm staying, that sprintf looks better 🤣

echo PHP_EOL;
echo sprintf('The percent error is: %s', $percentError);
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing ?> at the end, also should have 1 newline at end of file.

Copy link
Member Author

Choose a reason for hiding this comment

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

?> is discouraged and not needed since PHP 5.3 ( I think that version, not sure if 2/3/4 ).
I haven't left newline in any of previous PHP files, so this is with same style and format

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment about ?> was a mixup where I thought the code wasn't running because it didn't read a closing tag. It actually was running but was very slow.
The newline requirement is a git requirement rather than a php requirement iirc, Github warns that there is no newline at the end of file. I think it's because it messes with deltas of files.

4 changes: 4 additions & 0 deletions contents/monte_carlo_integration/monte_carlo_integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ each point is tested to see whether it's in the circle or not:
[import:1-4, lang:"ruby"](code/ruby/monte_carlo.rb)
{% sample lang="f90" %}
[import:1-8, lang:"fortran"](code/fortran/monte_carlo.f90)
{% sample lang="php" %}
[import:4-7, lang:"php"](code/php/monte_carlo.php)
{% endmethod %}

If it's in the circle, we increase an internal count by one, and in the end,
Expand Down Expand Up @@ -141,6 +143,8 @@ Feel free to submit your version via pull request, and thanks for reading!
[import, lang:"ruby"](code/ruby/monte_carlo.rb)
{% sample lang="f90" %}
[import, lang:"fortran"](code/fortran/monte_carlo.f90)
{% sample lang="php" %}
[import, lang:"php"](code/php/monte_carlo.php)
{% endmethod %}


Expand Down