Add Monte Carlo Integration in C# #309
Conversation
| System.Console.WriteLine($"The estimate of pi is: {output.PiEstimate}"); | ||
| System.Console.WriteLine($"The error is: {output.Error}"); | ||
|
|
||
| System.Console.WriteLine("How many samples should be used?"); |
There was a problem hiding this comment.
Should I leave this custom user input in?
|
|
||
| public Circle(double radius) => this.Radius = Math.Abs(radius); | ||
|
|
||
| public bool IsInMe(Point point) => point.X * point.X + point.Y * point.Y < Radius * Radius; |
There was a problem hiding this comment.
I used point.X * point.X now instead of using Math.Pow. It doesn't really matter here, what do you find cleaner?
There was a problem hiding this comment.
I would just leave Math.Pow ... it look then more compact and less chance of missreading
Gathros
added
the
Implementation
| } | ||
|
|
||
| var piEstimate = 4.0 * count / samples; | ||
| var error = Math.Abs(Math.PI - piEstimate); |
There was a problem hiding this comment.
Make the error a percentage (and add % when you print out the error) to remove ambiguity
| static void Main(string[] args) | ||
| { | ||
| var monteCarlo = new MonteCarlo(); | ||
| System.Console.WriteLine("Running with 10.000.000 samples."); |
There was a problem hiding this comment.
maybe it should be 10,000,000?
There was a problem hiding this comment.
You're right, just thought german there :D
There was a problem hiding this comment.
use 10 000 000 then or word notation 10 million
There was a problem hiding this comment.
I think 10,000,000 is fine since the example is in english.
| System.Console.WriteLine($"The estimate of pi is: {piEstimate.Estimate}"); | ||
| System.Console.WriteLine($"The error is: {piEstimate.Error}"); | ||
|
|
||
| System.Console.WriteLine("How many samples should be used?"); |
There was a problem hiding this comment.
I personally would not include this input part's to code examples. I would keep them minimal.
There was a problem hiding this comment.
I personally like to give the user a choice here, but I understand that it makes things quite a bit more messy.
june128
changed the title
…adable. Use percent error. Remove user input.
| } | ||
|
|
||
| var piEstimate = 4.0 * count / samples; | ||
| var percentError = (piEstimate - Math.PI) / Math.PI * 100; |
There was a problem hiding this comment.
One last thing: it feels strange to me to use Math.PIinside of a function that estimates pi. I've usually recommended to other people to calculate the error in main.
There was a problem hiding this comment.
Yeah, I see what you mean. I'm gonna change that.
|
Whoa, +4 -18 for moving a single line. OOP really is verbose :) |
|
@jiegillet I didn't need that output class anymore :D |
Since this PR is dead, I wrote it myself.
Math.Powinstead ofpoint.X * point.X.10,000,000instead of10.000.000for the console output.