patternjavaModerate
Pi by Monte-Carlo
Viewed 0 times
montecarlostackoverflow
Problem
I was inspired by this SO post to investigate
a good Java8 way to calculate Pi based on simulation.
I used a similar task to learn about parallel programming on both CUDA,
and Intel Xeon Phi processors. Those systems are more geared for
parallel programming, but I felt inclined to apply it to 'regular' Java
anyway.
Wikipedia has an small section showing
this, and
includes the following graphic:
The following code performs the above simulation:
```
import java.util.Arrays;
import java.util.Random;
/**
* Approximate the value of Pi by using a Monte-Carlo simulation for the area of a circle of radius 1.
*
* @author rolf
*
*/
public class PiGuess {
private static final ThreadLocal LOCAL_RANDOM = new ThreadLocal() {
protected Random initialValue() {
return new Random();
};
};
private static final int CPU_COUNT = Runtime.getRuntime().availableProcessors();
/**
* Split a number of samples as evenly as possible over the number of available processors.
* @param samples the samples to split
* @return an array containing the number of samples to process on each processor.
*/
private static final long[] apportion(final long samples) {
int core = CPU_COUNT;
final long[] portions = new long[core];
long remaining = samples;
while (core > 0) {
final long part = (remaining - 1 + core) / core;
core--;
portions[core] = part;
remaining -= part;
}
return portions;
}
/**
* Calculate the approximate area of a circle (radius 1.0) based on a sample system on a single quadrant of the circle.
* A parallel mechanism is used to improve performance.
* @param samples the number of samples to take
* @return the area of the circle.
*/
public static final double sampleCircleArea(final long samples) {
/*
Monte-Carlo simulation
a good Java8 way to calculate Pi based on simulation.
I used a similar task to learn about parallel programming on both CUDA,
and Intel Xeon Phi processors. Those systems are more geared for
parallel programming, but I felt inclined to apply it to 'regular' Java
anyway.
Wikipedia has an small section showing
this, and
includes the following graphic:
The following code performs the above simulation:
```
import java.util.Arrays;
import java.util.Random;
/**
* Approximate the value of Pi by using a Monte-Carlo simulation for the area of a circle of radius 1.
*
* @author rolf
*
*/
public class PiGuess {
private static final ThreadLocal LOCAL_RANDOM = new ThreadLocal() {
protected Random initialValue() {
return new Random();
};
};
private static final int CPU_COUNT = Runtime.getRuntime().availableProcessors();
/**
* Split a number of samples as evenly as possible over the number of available processors.
* @param samples the samples to split
* @return an array containing the number of samples to process on each processor.
*/
private static final long[] apportion(final long samples) {
int core = CPU_COUNT;
final long[] portions = new long[core];
long remaining = samples;
while (core > 0) {
final long part = (remaining - 1 + core) / core;
core--;
portions[core] = part;
remaining -= part;
}
return portions;
}
/**
* Calculate the approximate area of a circle (radius 1.0) based on a sample system on a single quadrant of the circle.
* A parallel mechanism is used to improve performance.
* @param samples the number of samples to take
* @return the area of the circle.
*/
public static final double sampleCircleArea(final long samples) {
/*
Monte-Carlo simulation
Solution
The code and documentation look fine in general, so this review will focus on minor optimizations on a per-method basis.
Use
You can set your
You need to give a
Reduce logic involved in
In my opinion too much is going on is this method, there needs to be a way to write it in a cleaner way. An example is the following, though it still does not eliminate all increment and decrement operations:
Streamify
This suggestion can be a matter of taste, but I believe a piece of code written in a functional programming variant is easier to understand than some calculations involving loops and more importantly it is less error-prone as you cannot possibly implement the
This solution also reduces the lines of code used, hence it should in theory be easier to maintain.
Use underscores in numeric literals
For clarify you might write the following in the
Note the
Use
ThreadLocal.withInitialYou can set your
LOCAL_RANDOM with the following:private static final ThreadLocal LOCAL_RANDOM = ThreadLocal.withInitial(Random::new);You need to give a
Supplier as argument, and that is exactly what Random::new is.Reduce logic involved in
apportionIn my opinion too much is going on is this method, there needs to be a way to write it in a cleaner way. An example is the following, though it still does not eliminate all increment and decrement operations:
/**
* Split a number of samples as evenly as possible over the number of available processors.
*
* This method will for example split 77 over 4 as [20, 19, 19, 19]
*
* @param samples the samples to split
* @return an array containing the number of samples to process on each processor.
*/
private static final long[] apportion(final long samples) {
int cores = CPU_COUNT;
long[] portions = new long[cores];
//calculate minimum amount of samples per core
long minSamples = samples / cores;
Arrays.setAll(portions, index -> minSamples);
//calculate remaining samples after evenly dividing the minimum amount over all cores
long remaining = samples - (minSamples * cores);
//split the remaining samples over the cores
for (int coreId = 0; remaining > 0; coreId++) {
portions[coreId]++;
remaining--;
}
return portions;
}Streamify
samplePortionThis suggestion can be a matter of taste, but I believe a piece of code written in a functional programming variant is easier to understand than some calculations involving loops and more importantly it is less error-prone as you cannot possibly implement the
count() method incorrectly!/**
* Internal sampling method that counts the number of input samples that are inside the circle too.
* @param samples the samples to calculate
* @return the count of samples that are inside the circle.
*/
private static final long samplePortion(final long samples) {
final Random random = LOCAL_RANDOM.get();
return LongStream.range(0, samples)
.filter(sample -> isInside(random))
.count();
}This solution also reduces the lines of code used, hence it should in theory be easier to maintain.
Use underscores in numeric literals
For clarify you might write the following in the
main method:double calc = sampleCircleArea(100_000);Note the
100_000 over 100000.Code Snippets
private static final ThreadLocal<Random> LOCAL_RANDOM = ThreadLocal.withInitial(Random::new);/**
* Split a number of samples as evenly as possible over the number of available processors.
*
* This method will for example split 77 over 4 as [20, 19, 19, 19]
*
* @param samples the samples to split
* @return an array containing the number of samples to process on each processor.
*/
private static final long[] apportion(final long samples) {
int cores = CPU_COUNT;
long[] portions = new long[cores];
//calculate minimum amount of samples per core
long minSamples = samples / cores;
Arrays.setAll(portions, index -> minSamples);
//calculate remaining samples after evenly dividing the minimum amount over all cores
long remaining = samples - (minSamples * cores);
//split the remaining samples over the cores
for (int coreId = 0; remaining > 0; coreId++) {
portions[coreId]++;
remaining--;
}
return portions;
}/**
* Internal sampling method that counts the number of input samples that are inside the circle too.
* @param samples the samples to calculate
* @return the count of samples that are inside the circle.
*/
private static final long samplePortion(final long samples) {
final Random random = LOCAL_RANDOM.get();
return LongStream.range(0, samples)
.filter(sample -> isInside(random))
.count();
}double calc = sampleCircleArea(100_000);Context
StackExchange Code Review Q#74780, answer score: 10
Revisions (0)
No revisions yet.