patternjavascriptMinor
Gravitational Brute Force N-body Algorithm
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
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
in both of the for loops you added an extra variable to hold the length of
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
Same thing here as well
and I would remove some of the vertical white space as well.
@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.
updateVelocityVectors that I couldn't quite figure out at firstupdateVelocityVectors: 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 thisfor (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.