vectr-ucla / direct_lidar_inertial_odometry

[IEEE ICRA'23] A new lightweight LiDAR-inertial odometry algorithm with a novel coarse-to-fine approach in constructing continuous-time trajectories for precise motion correction.
MIT License
530 stars 103 forks source link

Intended logic in updateKeyframes() #8

Closed juliangaal closed 1 year ago

juliangaal commented 1 year ago

I have some questions about this section of updateKeyframes():

https://github.com/vectr-ucla/direct_lidar_inertial_odometry/blob/a524000eb2e18a801a9222410650ddb12eddde39/src/dlio/odom.cc#L1572-L1589

To my understanding, there are some possible issues here:

rotational exploration keyframing

 // rotational exploration keyframing 
 if (abs(dd) <= this->keyframe_thresh_dist_ && abs(theta_deg) > this->keyframe_thresh_rot_ && num_nearby <= 1) { 
   newKeyframe = true; 
 } 

is always overwritten by

 if (abs(dd) <= this->keyframe_thresh_dist_) { 
   newKeyframe = false;
}

num_nearby has no effect

If this statement

 if (... || abs(theta_deg) > this->keyframe_thresh_rot_) { 
   newKeyframe = true; 
 } 

is true (from the second condition),

// rotational exploration keyframing 
if (abs(dd) <= this->keyframe_thresh_dist_ && abs(theta_deg) > this->keyframe_thresh_rot_ && num_nearby <= 1) { 
  newKeyframe = true; 
} 

this statement doesn't change newKeyFrame, even if num_nearby is larger than 1. At this point, abs(dd) <= this->keyframe_thresh_dist_ is always true, given that abs(dd) > this->keyframe_thresh_dist_ was false in the condition before.

Intended logic

What is the intended logic here? I couldn't find much about this in your papers, except the adaptive thresholding distance to account for tight spaces.

Without the rotational exploration keyframing, the logic is clear to me (roughly):

if abs(dd) <= this->keyframe_thresh_dist or abs(dd) > 0.5 -> return false
else if abs(dd) > this->keyframe_thresh_dist -> return true

Now, I don't quite understand why the keyframe are updated when a large rotation was registered. In a scenario where, let's say, the robot stays in the same spot, but turns 180deg. Why do I have to update keyframes? The map is rotation invariant, so to speak, so I have gained no new information about the environment just be turning. I guess this is what was meant with the following statement in the DLO-paper?

Keyframe nodes are commonly dropped using fixed thresholds (e.g., every 1m or 10◦ of translational or rotational change)

Thanks for you help, happy to discuss

kennyjchen commented 1 year ago

Ah -- the entire "check for nearby keyframes" statement should not be there. There were originally some additional checks in that block from our new DLIOM algorithm that I removed when open-sourcing this code (e.g., slippage), but I should have just removed that entire block. I'll push a new update in a bit. Thanks for the catch.

Regarding whether keyframes should be updated after a large rotation -- that largely depends on the FOV of the LiDAR. If the sensor is 360 degrees, then yes, keyframes are rotation-invariant. There are some sensors (i.e., Intel L515 or some older Livox sensors) that have a limited FOV, so keyframes are rotation-dependent. Even with modern mechanical LiDAR, they are only 360 FOV in the x-y direction, so tilting up or down would provide more information (and hence need a new keyframe). If I recall correctly, this exact scenario can be seen in the beginning of one of the LIO-SAM datasets.

juliangaal commented 1 year ago

There were originally some additional checks in that block from our new DLIOM algorithm that I removed when open-sourcing this code (e.g., slippage), but I should have just removed that entire block. I'll push a new update in a bit. Thanks for the catch.

Understood. DLIOM being available some day would be awesome :)

There are some sensors (i.e., Intel L515 or some older Livox sensors) that have a limited FOV, so keyframes are rotation-dependent [...] so tilting up or down would provide more information

Makes perfect sense!

kennyjchen commented 1 year ago

Updated! Thanks again for the catch.