davidrmiller / biosim4

Biological evolution simulator
Other
3.1k stars 435 forks source link

Optimise basicTypes.cpp #29

Closed Asa-Hopkins closed 2 years ago

Asa-Hopkins commented 2 years ago

I've cleaned up most functions in basicTypes.cpp, a few were worth improving for performance reasons but I thought I'd clean up the others whilst I was at it. Most of them weren't performance bottlenecks, though their replacements are at least as performant and as accurate under reasonable constraints.

Rotations are now done through a lookup table with index dir*8 + n%7. This removes the need for looping or checks of any kind, and rotations outside of [0,7] are now handled too. This is around 30-50% faster depending on the situation, which cuts 2-3% off the running time of the program.

Converting a Dir to a Coord is now done by lookup too, this is around 50% faster, cutting an incredible 0.5% off the running time.

asDir has been overhauled to remove the atan2 call and floating point arithmetic. The new method uses the idea that there are 4 lines which bisect the angles between coordinate directions, and by checking which side of those lines a point is, it can be determined which direction it is closest to. These lines are like y = (1+sqrt(2))*x which would be messy to work with, instead rotate the whole system 22.5 degrees so the lines become y=x, y=-x, y=0 and x=0. This method is around 20x faster than the original, and accurate for all possible Coord inputs (tested exhaustively). I had to switch the original implementation's floats for doubles for testing as there were some points that it classified incorrectly, but after that the two methods agreed.

asCoord has also been overhauled to remove the trigonometric functions and the floating point arithmetic. It is around twice as fast, and the output matches with the original for all 9 coordinate directions, and for values of mag in [-46341,46341], since this is the largest mag can be and still be in bounds.

Asa-Hopkins commented 2 years ago

I'm a bit confused how I wasn't encountering that compiler error, but to fix it I added some #define blocks to let me write compass directions in shorthand, without the BS::Compass prefix. It's a little messy but it keeps the lookup tables legible at least.

davidrmiller commented 2 years ago

The #defines are unfortunate, but might be the best solution for those tables. Maybe those short names could be written as const definitions to avoid using the preprocessor while still controlling their scope. I look forward to looking at your optimizations in more detail after a bit more work on a testing strategy. You have a good eye for math optimizations.

Asa-Hopkins commented 2 years ago

I'm unsure how I missed that, I've replaced them with defining constexpr values instead, which I'm a lot happier with.