HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Gravitational Brute Force N-body Algorithm

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
forcegravitationalalgorithmbrutebody

Problem

I've just started dabbling with code, and as a learning exercise I've written a simple algorithm for solving gravitational n-body problems numerically in JavaScript.

I would be very grateful for general feedback as far as the quality of the code goes (first time I'm asking somebody to review my code; I'm very shy about it!), and more specifically advise on how I could optimize it as when I implement the algorithm in THREE.js in the shape of a simulation of the solar system the simulation starts lagging when I include more than around 70 bodies in the simulation. I know n-body algorithms are computationally intensive by nature, especially those of the brute force variety, but I'm pretty sure I've missed out on some useful tricks that would push it a bit further.

Update: to see a working example of the code, check out the jsfiddle I have thrown together https://jsfiddle.net/e9t88dwh

```
function nBodyProblem(parameters) {

this.g = parameters.g; //Gravitational constant
this.law = parameters.law; //Force law, inverse-square in our universe
this.dt = parameters.dt; //Time step to be used
this.masses = parameters.masses; //Object array with initial conditions for the bodiess to be simulated

}

nBodyProblem.prototype = {

constructor: nBodyProblem,

getDistance: function (m1, m2) {

return Math.pow((m1.x - m2.x) (m1.x - m2.x) + (m1.y - m2.y) (m1.y - m2.y) + (m1.z - m2.z) * (m1.z - m2.z), this.law);

},

updatePositionVectors: function () {

for (var i = 0, len = this.masses.length; i < len; i++) {

this.masses[i].x += this.masses[i].vx * this.dt;
this.masses[i].y += this.masses[i].vy * this.dt;
this.masses[i].z += this.masses[i].vz * this.dt;
}

return this;

},

updateVelocityVectors: function () {

var forceVectorX = 0;
var forceVectorY = 0;
var forceVectorZ = 0;

for (var i = 0, ilen = this.masses.length; i < ilen; i++) {

for

Solution

you did something in updateVelocityVectors that I couldn't quite figure out at first

updateVelocityVectors: function () {

    var forceVectorX = 0;
    var forceVectorY = 0;
    var forceVectorZ = 0;

    for (var i = 0, ilen = this.masses.length; i < ilen; i++) {

        for (var j = 0, jlen = this.masses.length; j < jlen; j++) {

            //We don't want to calculate self gravity!!!!! 

            if (i !== j) {

                forceVectorX += (this.g * this.masses[j].m) * (this.masses[j].x - this.masses[i].x) / this.getDistance(this.masses[i], this.masses[j]);
                forceVectorY += (this.g * this.masses[j].m) * (this.masses[j].y - this.masses[i].y) / this.getDistance(this.masses[i], this.masses[j]);
                forceVectorZ += (this.g * this.masses[j].m) * (this.masses[j].z - this.masses[i].z) / this.getDistance(this.masses[i], this.masses[j]);

            }

        }

        this.masses[i].vx += forceVectorX * this.dt;
        this.masses[i].vy += forceVectorY * this.dt;
        this.masses[i].vz += forceVectorZ * this.dt;

        forceVectorX = 0;
        forceVectorY = 0;
        forceVectorZ = 0;

    }

    return this;

}


in both of the for loops you added an extra variable to hold the length of this.masses, but never use it anywhere else in the loop. I think that you shouldn't declare the variable at all and just use this.masses.length in the declaration of both for loops like this

for (var j = 0, j < this.masses.length; j++) {


or an even better idea would be to assign the value to a variable outside of both loops so that you only call the length property of this.masses once, like this.

var massesLength = this.masses.length;
for (var i = 0; i < massesLength; i++) {
    for (var j = 0; j < massesLength; j++) {
    ...


Same thing here as well

updatePositionVectors: function () {

    for (var i = 0, len = this.masses.length; i < len; i++) {

        this.masses[i].x += this.masses[i].vx * this.dt;
        this.masses[i].y += this.masses[i].vy * this.dt;
        this.masses[i].z += this.masses[i].vz * this.dt;
    }

    return this;

},


and I would remove some of the vertical white space as well.

updatePositionVectors: function () {
    var massesLength = this.masses.length;
    for (var i = 0; i < massesLength; i++) {
        this.masses[i].x += this.masses[i].vx * this.dt;
        this.masses[i].y += this.masses[i].vy * this.dt;
        this.masses[i].z += this.masses[i].vz * this.dt;
    }
    return this;
},


@Ismael Miguel is correct, by using the length property inside the for declaration you would access that property every loop, which would make this loop very inefficient, so create the variable outside of the for loop.

Code Snippets

updateVelocityVectors: function () {

    var forceVectorX = 0;
    var forceVectorY = 0;
    var forceVectorZ = 0;

    for (var i = 0, ilen = this.masses.length; i < ilen; i++) {

        for (var j = 0, jlen = this.masses.length; j < jlen; j++) {

            //We don't want to calculate self gravity!!!!! 

            if (i !== j) {

                forceVectorX += (this.g * this.masses[j].m) * (this.masses[j].x - this.masses[i].x) / this.getDistance(this.masses[i], this.masses[j]);
                forceVectorY += (this.g * this.masses[j].m) * (this.masses[j].y - this.masses[i].y) / this.getDistance(this.masses[i], this.masses[j]);
                forceVectorZ += (this.g * this.masses[j].m) * (this.masses[j].z - this.masses[i].z) / this.getDistance(this.masses[i], this.masses[j]);

            }

        }

        this.masses[i].vx += forceVectorX * this.dt;
        this.masses[i].vy += forceVectorY * this.dt;
        this.masses[i].vz += forceVectorZ * this.dt;

        forceVectorX = 0;
        forceVectorY = 0;
        forceVectorZ = 0;

    }

    return this;

}
for (var j = 0, j < this.masses.length; j++) {
var massesLength = this.masses.length;
for (var i = 0; i < massesLength; i++) {
    for (var j = 0; j < massesLength; j++) {
    ...
updatePositionVectors: function () {

    for (var i = 0, len = this.masses.length; i < len; i++) {

        this.masses[i].x += this.masses[i].vx * this.dt;
        this.masses[i].y += this.masses[i].vy * this.dt;
        this.masses[i].z += this.masses[i].vz * this.dt;
    }

    return this;

},
updatePositionVectors: function () {
    var massesLength = this.masses.length;
    for (var i = 0; i < massesLength; i++) {
        this.masses[i].x += this.masses[i].vx * this.dt;
        this.masses[i].y += this.masses[i].vy * this.dt;
        this.masses[i].z += this.masses[i].vz * this.dt;
    }
    return this;
},

Context

StackExchange Code Review Q#120817, answer score: 6

Revisions (0)

No revisions yet.