KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
686 stars 228 forks source link

lookdirup() misbehaves for big vectors #1695

Open sshilovsky opened 8 years ago

sshilovsky commented 8 years ago
print solar_dir.
V(0, 13290119712.7574, 0)
print solar_dir_top.
V(3.83541842173846E+19, 0, -1.76627130112357E+20)
print lookdirup(solar_dir, solar_dir_top).
R(270,0,0)
print lookdirup(solar_dir:normalized, solar_dir_top:normalized).
R(270,347.748,0)

the two R() outputs should be equal. according to my task, the latter value is actually correct.

also, solar_dir and solar_dir_top are orthogonal here.

It might make sense to request normalized vectors in parameters for lookdirup(); this should be reflected in the docs though.

Dunbaratu commented 8 years ago

This might have something to do with the fact that under the hood, we're calling into the Unity engine using its floating point (single precision) rotations rather than its double precision ones.

The biggest integer value that can be stored in a single-float before you hit the point where you can't store consecutive integers any more (each consecutive value is more than 1.0 away from its previous neighbour value) is 2^24, or 16777216. The magnitudes you're dealing with are much larger than that. It may have less to do with getting the vector magnitude down to 1.0 and more to do with just the general reduction in magnitude down into any low value range where it can be precisely stored.

For example, can you test it by doing something like this instead?

set big_num to 10000000000.
set smaller_solar_dir to
    V(solar_dir:x / big_num,
      solar_dir:y / big_num,
      solar_dir:z / big_num).
set bigger_num to 100000000000000000.
set smaller_solar_dir_top to
    V(solar_dir_top:x / bigger_num, 
      solar_dir_top:y / bigger_num, 
      solar_dir_top:z / bigger_num). 
print lookdirup(smaller_solar_dir, smaller_solar_dir_top).

That will test whether it specifically has to be normalized for it to work like the issue title suggests, or if it merely has to be smaller for it to work and normalizing it only worked because it happens to make it smaller.

sshilovsky commented 8 years ago

lookdirup(solar_dir, smaller_solar_dir_top) works fine.

Dunbaratu commented 8 years ago

Okay, then I think this issue is just a specific case example of the problem already described in issue #1663. Just making a note that we should make a special note to test this case when fixing #1663 because here we "pass through" single precision on our way TO the Quaternion before we get to it, by way of converting the Vector (that uses doubles) into a Vector3d (that uses doubles) into a Vector3 (that uses floats!!) before the Quaternion gets called.