patterncppMinor
Calculating pi by adding areas of thin rectangles
Viewed 0 times
rectanglesaddingcalculatingareasthin
Problem
I wrote a small program for fun to try to prove Pi by taking a certain precision and radius and using it to calculate the area of the circle. My method should be giving me an area that is just slightly larger than the actual area of the circle, approaching the actual area as the precision gets smaller.
The funny thing is that this area should always be larger than the actual area, but when I set the precision to be less than 1.0e-9L (say 1.0e-10L), I get a result smaller than 3.141592654. Can you look at my code and tell me where I messed up if anywhere?
```
#include
#include
#include
#include
#include
using namespace std;
namespace Settings{
register constexpr long double precision = 1.0e-10L;
register constexpr long double radius = 1.0e0L;
}
// Make the math cleaner
inline long double SQUARE(register long double Val) { return Val * Val; }
// Main calculate function
long double get_Area(register long double radius = Settings::radius);
int main()
{
time_t benchmark = time(0);
register long double area = get_Area();
register long double pi = area / SQUARE(Settings::radius);
cout A1 = async(launch::async, part, 0.0L, (1.0L / 8.0L) * radius, max_Area);
future A2 = async(launch::async, part, (1.0L / 8.0L) radius, (2.0L / 8.0L) radius, max_Area);
future A3 = async(launch::async, part, (2.0L / 8.0L) radius, (3.0L / 8.0L) radius, max_Area);
future A4 = async(launch::async, part, (3.0L / 8.0L) radius, (4.0L / 8.0L) radius, max_Area);
future A5 = async(launch::async, part, (4.0L / 8.0L) radius, (5.0L / 8.0L) radius, max_Area);
future A6 = async(launch::async, part, (5.0L / 8.0L) radius, (6.0L / 8.0L) radius, max_Area);
future A7 = async(launch::async, part, (6.0L / 8.0L) radius, (7.0L / 8.0L) radius, max_Area);
future A8 = async(launch::async, part, (7.0L / 8.0L) * radius, radius, max_Area);
area = A1.get() + A2.get() + A3.get() + A4.get() + A5.get
The funny thing is that this area should always be larger than the actual area, but when I set the precision to be less than 1.0e-9L (say 1.0e-10L), I get a result smaller than 3.141592654. Can you look at my code and tell me where I messed up if anywhere?
```
#include
#include
#include
#include
#include
using namespace std;
namespace Settings{
register constexpr long double precision = 1.0e-10L;
register constexpr long double radius = 1.0e0L;
}
// Make the math cleaner
inline long double SQUARE(register long double Val) { return Val * Val; }
// Main calculate function
long double get_Area(register long double radius = Settings::radius);
int main()
{
time_t benchmark = time(0);
register long double area = get_Area();
register long double pi = area / SQUARE(Settings::radius);
cout A1 = async(launch::async, part, 0.0L, (1.0L / 8.0L) * radius, max_Area);
future A2 = async(launch::async, part, (1.0L / 8.0L) radius, (2.0L / 8.0L) radius, max_Area);
future A3 = async(launch::async, part, (2.0L / 8.0L) radius, (3.0L / 8.0L) radius, max_Area);
future A4 = async(launch::async, part, (3.0L / 8.0L) radius, (4.0L / 8.0L) radius, max_Area);
future A5 = async(launch::async, part, (4.0L / 8.0L) radius, (5.0L / 8.0L) radius, max_Area);
future A6 = async(launch::async, part, (5.0L / 8.0L) radius, (6.0L / 8.0L) radius, max_Area);
future A7 = async(launch::async, part, (6.0L / 8.0L) radius, (7.0L / 8.0L) radius, max_Area);
future A8 = async(launch::async, part, (7.0L / 8.0L) * radius, radius, max_Area);
area = A1.get() + A2.get() + A3.get() + A4.get() + A5.get
Solution
I see a number of things that could help you improve your program.
Don't abuse
Putting
Don't use
There are two reasons not to use
Don't use
As of C++11, the use of the
Be careful with floating point
Adding a small number to a large number using floating point is risky because in certain cases, as potentially with your
Consider using portable scalability
The choice of 8 parts is just fine, but you might consider using
Don't repeat yourself
Rather than calculating the low and high values for each slice manually, and naming each
Extend the accuracy
Since you're using a simple Riemann sum, on the quarter circle you've chosen the sum will indeed always be greater than the actual value. However, we can very simply take advantage of the symmetry of the problem and have that error mostly cancel out by summing the entire top half of the circle. That is, instead of $$\pi = 4\int_0^{r} \sqrt{r^2 - x^2}\delta x$$ we can instead compute $$\pi = 2\int_{-r}^{r} \sqrt{r^2 - x^2}\delta x$$.
Omit
When a C++ program reaches the end of
Don't abuse
using namespace stdPutting
using namespace std at the top of every program is a bad habit that you'd do well to avoid. Know when to use it and when not to (as when writing include headers). In this particular case, I happen to think it's perfectly appropriate because it's a single short program that and not a header. Some people seem to think it should never be used under any circumstance, but my view is that it can be used as long as it is done responsibly and with full knowledge of the consequences. Don't use
system("pause")There are two reasons not to use
system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example:void pause() {
getchar();
}Don't use
registerAs of C++11, the use of the
register keyword is deprecated. In C++17, it will be gone, so it's wise to avoid its use. Be careful with floating point
Adding a small number to a large number using floating point is risky because in certain cases, as potentially with your
for loop within get_Area(). Specifically, with floating point numbers, adding a large number to a small number can lead to loss of precision. In extreme cases, if a is very large and b is very small, we can have a + b = a which is obviously incorrect for non-zero values of b. (See the oft-cited and still excellent What Every Computer Scientist Should Know About Floating-Point Arithmetic, by David Goldberg for a readable technical discussion for why this is so.) I should mention that it will not happen with this particular code with these particular values, but it's a potential problem that we programmers should all understand.Consider using portable scalability
The choice of 8 parts is just fine, but you might consider using
std::thread::hardware_concurrency for a portable way to scale on multiprocessor systems.Don't repeat yourself
Rather than calculating the low and high values for each slice manually, and naming each
std::future by hand, have the computer do it instead:const unsigned slices = thread::hardware_concurrency();
vector> A(slices);
const long double increment = radius / slices;
long double lo = 0;
long double hi = increment;
for (unsigned i=0; i &f){
return accum + f.get();
}) * 4;Extend the accuracy
Since you're using a simple Riemann sum, on the quarter circle you've chosen the sum will indeed always be greater than the actual value. However, we can very simply take advantage of the symmetry of the problem and have that error mostly cancel out by summing the entire top half of the circle. That is, instead of $$\pi = 4\int_0^{r} \sqrt{r^2 - x^2}\delta x$$ we can instead compute $$\pi = 2\int_{-r}^{r} \sqrt{r^2 - x^2}\delta x$$.
Omit
return 0When a C++ program reaches the end of
main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.Code Snippets
void pause() {
getchar();
}const unsigned slices = thread::hardware_concurrency();
vector<future<long double>> A(slices);
const long double increment = radius / slices;
long double lo = 0;
long double hi = increment;
for (unsigned i=0; i<slices; ++i) {
A[i] = async(launch::async, part, lo, hi, max_Area);
lo += increment;
hi += increment;
}
return std::accumulate(A.begin(), A.end(), 0.0L,
[](long double accum, std::future<long double> &f){
return accum + f.get();
}) * 4;Context
StackExchange Code Review Q#126570, answer score: 6
Revisions (0)
No revisions yet.