celestiary / web

Astronomical simulator of solar system and local stars
https://celestiary.github.io/
42 stars 2 forks source link

Not quite an inverse square #16

Closed won3d closed 3 years ago

won3d commented 3 years ago

https://github.com/pablo-mayrgundter/celestiary/blob/5c8e5f2b32272cf07a95f450987bb16596acbba8/js/Galaxy.js#L99-L103

Here is a subtle thing that happens because it is easy to confuse how to apply the vector length. On one hand, you have to normalize the dXYZ vector, which is in addition to the inverse square law. The terseness of mathematics makes this an easy trap to fall into. But what this actually means is you have to divide the un-normalized dXYZ by length(dXYZ)^3, if I have that right.

A few notes on implementation. You could try to fold the powers and the divide into a single pow and never explicitly compute the length. Something like:

const g = G * Math.pow(dX*dX + dY*dY + dZ*dZ, -1.5);

However, it is worth mentioning that, for various reasons due to its ubiquity and nice mathematical properties, reciprocal square root is a very common instruction on CPUs, and pow might not turn into anything efficient. This might be a better sequence:

const n = 1.0 / Math.sqrt(dX*dX + dY*dY + dZ*dZ);
const g = n * n * n;

As a result of these changes, you'll likely find it is much harder to get a stable galaxy, and it will be very easy for points to slingshot each other into the void. It might be necessary to add a little epsilon to avoid dividing by zero (or close to zero). If I remember an epsilon of around ~0.001 worked OK, but I think it is probably G dependent. You'd stick that wherever you are computing length squared: dX*dX + dY*dY + dZ*dZ + 0.001.

pablo-mayrgundter commented 3 years ago

Huh, the second way, Math.pow(..., 1.5) is crazy slow. 370ms per frame, vs 45ms for existing code! Math.sqrt is slightly faster than the old way, 43ms. Maybe same code under the hood with overhead from the indirection.

Indeed, the cubic is throwing things into the void.. it's also crunching everything into a vertical column. Hmm.

On Mon, Jan 11, 2021 at 5:28 PM won3d notifications@github.com wrote:

https://github.com/pablo-mayrgundter/celestiary/blob/5c8e5f2b32272cf07a95f450987bb16596acbba8/js/Galaxy.js#L99-L103

Here is a subtle thing that happens because it is easy to confuse how to apply the vector length. On one hand, you have to normalize the dXYZ vector, which is in addition to the inverse square law. The terseness of mathematics makes this an easy trap to fall into. But what this actually means is you have to divide the un-normalized dXYZ by length(dXYZ)^3, if I have that right.

A few notes on implementation. You could try to fold the powers and the divide into a single pow and never explicitly compute the length. Something like:

const g = G Math.pow(dXdX + dYdY + dZdZ, -1.5);

However, it is worth mentioning that, for various reasons due to its ubiquity and nice mathematical properties, reciprocal square root is a very common instruction on CPUs, and pow might not turn into anything efficient. This might be a better sequence:

const n = 1.0 / Math.sqrt(dXdX + dYdY + dZdZ); const g = n n * n;

As a result of these changes, you'll likely find it is much harder to get a stable galaxy, and it will be very easy for points to slingshot each other into the void. It might be necessary to add a little epsilon to avoid dividing by zero (or close to zero). If I remember an epsilon of around ~0.001 worked OK, but I think it is probably G dependent. You'd stick that wherever you are computing length squared: dXdX + dYdY + dZ*dZ + 0.001.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pablo-mayrgundter/celestiary/issues/16, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS5V34ADH3CR4WNWJQMDCDSZOCQVANCNFSM4V6LL2BQ .

-- interweb homepage http://sites.google.com/site/pablomayrgundter

won3d commented 3 years ago

v8 probably specializes pow for some easy exponents, but -1.5seems to not one of them.

I don't know about the vertical column issue, but you'll definitely have to change G.

pablo-mayrgundter commented 3 years ago

Also fixed in https://github.com/pablo-mayrgundter/celestiary/commit/36f4387bd4d4ce08a4f5865bc1ec9e56d57b1e08#diff-1ab10843cfe0c451bbbd62dc8ec9cc22d33e1e1ac812538d3bb4aea3f7906d5bR94