-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mass values go below zero #30
Comments
Should there be a clamp? |
There is a clamp. But even with the current epsilon of 0.01, the error still goes beyond that on larger maps (1024+) with 20+ plates. To top it off, there could be a bug somewhere and it would be extremely difficult to track down. |
I've identified one source of unnecessary error. The function erode() calculates the mass of the crust before performing some float multiplications on the crust. This should be changed to do the mass calculation at the end of the erosion loop! |
Even after changing the mass calculation to the end of erode(), I still get negative mass values beyond the epsilon. There are only additions and subtractions to mass except for in the erosion code, which recalculates mass. Because of this, I can think of no better scenario for fixed point numbers. I strongly suggest that we change all heightmap data to signed 32-bit fixed point numbers with 24 or 25 bits of decimal precision. This gives 7 decimal places of precision. Mass should be a signed 64-bit fixed point number with a decimal precision that exactly matches the heightmap and the rest goes to the integer part. |
I tried fixing the negative mass issues with no luck, it is not trivial. I think that using integers would also make easier to reproduce results across platforms. It would just be a big task :) |
This is because of floating point inaccuracies building up over MANY mass additions (determined by the length and size of the simulation). The code uses the crust height to determine mass. Both of these values are stored as floats. I've tried changing it to a double, but even I saw the inaccuracy build up fairly badly. The only other thing I can think of is to change these to fixed point numbers. If this option was chosen, it looks to be a big change that will require lots of testing.
The text was updated successfully, but these errors were encountered: