3
\$\begingroup\$

I have created a program that calculates pi using a Monte Carlo method. It also animates the process and displays the value as it is updated to show how it gets closer and closer to the actual value as more time passes. It is made with HTML canvas. I would like some feedback on my program.

"use strict";


function hypot(a, b) {
  return Math.sqrt(a * a + b * b);
}

function randInt(min, max) {
  return Math.floor(Math.random() * (max - min)) + min
}

class Circle {
  constructor(x, y, r) {
    this.x = x;
    this.y = y;
    this.r = r;
  }

  draw() {
    ctx.strokeStyle = "#000";
    ctx.lineWidth = "5px";
    ctx.beginPath();
    ctx.arc(this.x, this.y, this.r, 0, Math.PI * 2);
    ctx.stroke();
  }

  collidePoint(pt) {
    var dx = this.x - pt.x;
    var dy = this.y - pt.y;
    return hypot(dx, dy) <= this.r;
  }
}

class Point {
  constructor(x, y) {
    this.x = x;
    this.y = y;
  }

  draw() {
    ctx.fillStyle = "rgba(0, 0, 0, 16)";
    ctx.fillRect(this.x, this.y, 1, 1);
  }
}


const canvas = document.getElementById("disp");
const ctx = canvas.getContext("2d");
canvas.width = window.innerWidth;
canvas.height = window.innerHeight;
var width = parseInt(canvas.width);
var height = parseInt(canvas.height);
var trys = 0;
var lands = 0;
var area = width * height;
var points = [];
var ball = new Circle(width / 2, height / 2, Math.min(width, height) / 2);
setInterval(step, 10);

function drawAll() {
  ctx.clearRect(0, 0, width, height);
  ball.draw();
  points.forEach((point) => {
    point.draw();
  });
}

function step() {
  drawAll();
  var pnt = new Point(randInt(0, width), randInt(0, height));
  trys++;
  if (ball.collidePoint(pnt)) {
    lands++;
  }
  var prob = lands / trys;
  var pi = (prob * area) / Math.pow(ball.r, 2);
  document.getElementById("out").innerText = pi.toString();
  points.push(pnt);
}
html, body {
  height: 100%;
  overflow: hidden;
}

#disp {
  width: 100vw;
  height: 100vh;
  margin: 0;
}

#box {
  position: absolute;
  z-index: 1;
  width: 200px;
  height: 20px;
  left: 5%;
  top: 5%;
  background-color: whitesmoke;
}
<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width">
    <title>Monte Carlo Pi</title>
    <link href="style.css" rel="stylesheet" type="text/css" />
  </head>
  <body>
    <canvas id="disp"></canvas>
    <span id="box">Pi &#x2248; <span id="out"></span></span>
    <script src="script.js"></script>
  </body>
</html>

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The jungle

OO design tends to create unneeded eco systems when the focus is the objects not the task at hand.

In this case Point, Circle, points and supporting functions have nothing to do with the problem. They exist to serve each other.

You clear the canvas so you can draw all the points, seams to me that the only reason you keep an array of points is so you can draw them after you clear the canvas.

Storing the points means no machine today using your function will ever be able to solve PI to over 6 digits as none have either the memory to hold all the points, nor the time to draw them.

Having an open setInterval (no handle to stop the interval) constitutes a memory leak because you push to the points array each time.

The task is to calculate Pi via Monte Carlo method and show the points, it can be done without the overhead,of the OO eco system and supporting scope.

General point

  • Use const for constants.

  • ECMAScripts Math has a hypot method already, however it is much quicker to do the calculation inline. As the calculation requires the radius squared, you may as well store the radius as such and then test if inside with x * x + y * y < radiusSqr

  • window is the default object, you don't use it for window.parseInt so why use it for innerWidth?

  • canvas.width, and canvas.height represent the resolution of the canvas. They can only be integer values. parseInt is thus redundant.

  • You are moving the ball to the center of the canvas and with it all the points. As such need to move each point back to the origin to calculate its distance from the ball. If you moved the canvas origin to the canvas center you only need the radius, and you don't need to move the points to the ball origin to get the distance.

  • The calculation works for random distribution of points. You introduce a very non uniform distribution by rounding the points to the nearest pixel.

  • Every 10 ms you redraw the canvas content. All browsers have a MAX display rate of 60 frame per second. you draw 100 so 40 of the frames you draw are never seen. Never render to the DOM or to canvas using setInterval at rates above a few frames per second. Use requestAnimationFrame

  • You Search the DOM for an element with id "out". every 10 ms. Better to get it once at the start. better yet use direct reference via the id See example

  • You call the Number.toString when you set out.textContent ECMAScript does type coercion automatically you don't need to call toString when assign a number to a string

  • Your DOM element ids are rather poor and could be improved

  • You should set the body margin and padding to "0px" so that the canvas fits without needing to have overflow clip the canvas.

  • If you don't need to locate an element uniquely use a CSS class to set its style rather than id

  • Use the setTimeout rather than setInterval to give a interval between rather than an interval over. Many devices need time to cool or they will force the issue and reduce the clock speed. setInterval can result in back to back calls to the function giving no rest for the CPU/GPU, the result will be slower performance for all services the machine is running.

Example

The example removes the overhead of storing points.

The constant pointsPerInterval defines how many points to calculate each interval. The points are added to the current 2D context path and only rendered when it can be displayed using requestAnimationFrame

Having high rated of points quickly results in a black canvas. So the number of points drawn per frame is only a sub set of the points used. Thus it draws only ~1000 points per second and uses ~10million points per second to calculate PI.

The example also exposes a simple interface to reset the animations, or stop.

Also shows number of points use. "Mp" represents Mega points (1,000,000 points)

Click the canvas to reset.

"use strict";
const calculatePi = (() => {
    const ctx = canvas.getContext("2d");
    const width = canvas.width = innerWidth, halfWidth = width / 2;
    const height = canvas.height = innerHeight, halfHeight = height / 2;
    const radiusSqr = Math.min(halfWidth, halfHeight) ** 2;
    const areaScale = width * height / radiusSqr;
    const pointsPerInterval = 100000, interval = 10, showPointCount = 10;
    var pointCount, inCircleCount, timeoutHandle;

    ctx.setTransform(1, 0, 0, 1, halfWidth, halfHeight); // Move origin to cent of canvas

    resetCanvas();
    requestAnimationFrame(render); 
    function resetCanvas() {
        pointCount = 0;
        inCircleCount = 0;
        ctx.clearRect(-halfWidth, -halfHeight, width, height);
        ctx.lineWidth = 5;
        ctx.beginPath();
        ctx.arc(0, 0, radiusSqr ** 0.5, 0, Math.PI * 2);
        ctx.stroke();
        ctx.beginPath();
        addPoints();
    }
    function render(){
        ctx.fill();
        ctx.beginPath();
        currentPiId.innerText = inCircleCount / pointCount * areaScale;
        pointCountId.innerText = pointCount / 1000000 | 0;
        requestAnimationFrame(render);
    }
    function addPoints() {
        var i = pointsPerInterval; 
        pointCount += pointsPerInterval;
        while (i --) {
            const x = -halfWidth + Math.random() * width;
            const y = -halfHeight + Math.random() * height;
            i < showPointCount && (ctx.rect(x, y, 1, 1));
            inCircleCount += x * x + y * y < radiusSqr ? 1 : 0;
        }
        timeoutHandle = setTimeout(addPoints, interval);
    }   
    return Object.freeze({
        stop() { clearTimeout(timeoutHandle) },
        restart() { 
            this.stop();
            resetCanvas();
        },
    });
})();


canvas.addEventListener("click", () => calculatePi.restart());
#canvas  {
  position: absolute;
  top: 0px;
  left: 0px;  
}
 
.box {
  position: absolute;
  z-index: 1;
  /*width: 200px;*/
  height: 20px;
  left: 5%;
  top: 5%;
  background-color: whitesmoke;
}
<canvas id="canvas"></canvas>
<span class="box"><span id="pointCountId"></span>Mp = Pi &#x2248; <span id="currentPiId"></span></span>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.