NeoGeographyToolkit / StereoPipeline

The NASA Ames Stereo Pipeline is a suite of automated geodesy & stereogrammetry tools designed for processing planetary imagery captured from orbiting and landed robotic explorers on other planets.
Apache License 2.0
478 stars 168 forks source link

bundle_adjust option for random offset amplitude to be used if --num-random-passes>0 #344

Closed steo85it closed 2 years ago

steo85it commented 2 years ago

Is your feature request related to a problem? Please describe. The --num-random-passes option of the (parallel_)_bundle_adjust ASP function applies a randomized offset with a fixed std of 5 meters to the cameras at each pass. That might not be enough to avoid local minima, depending on the problem and the resolution of the images. For reference, see: src/Tools/bundle_adjust.cc, l.1509 src/Tools/bundle_adjust_misc_functions.h, l.182

/// Apply a random offset to each camera position.                                                                                                       
  void randomize_cameras() {                                                                                                                               
    **// These are stored as x,y,z, axis_angle.                                                                                                              
    // - We move the position +/- 5 meters.                                                                                                                
    // - Currently we don't adjust the angle.**                                                                                                              
    boost::random::uniform_int_distribution<> xyz_dist(0, 10);                                                                                             
    const size_t NUM_CAMERA_PARAMS = 6;                                                                                                                    
    VW_ASSERT((m_cameras_vec.size() % NUM_CAMERA_PARAMS) == 0,                                                                                             
                LogicErr() << "Camera parameter length is not a multiple of 6!");                                                                          
    const size_t num_cameras = m_cameras_vec.size() / NUM_CAMERA_PARAMS;                                                                                   
    for (size_t c=0; c<num_cameras; ++c) {                                                                                                                 
      double* ptr = get_camera_ptr(c);                                                                                                                     
      for (size_t i=0; i<3; ++i) {                                                                                                                         
        **int o = xyz_dist(m_rand_gen) - 5;**                                                                                                                  
        ptr[i] += o;                                                                                                                                       
      }                                                                                                                                                    
      //for (size_t i=3; i<PARAMS_PER_CAM; ++i) {                                                                                                          
      //}                                                                                                                                                  
    }                                                                                                                                                      
  }    

Describe the solution you'd like It would be great to have an option for (parallel_)bundle_adjust (e.g., --random-offset-size) so that the user could provide the amplitude of the applied random offsets.

Describe alternatives you've considered Hard-coding a custom value for each application at int o = xyz_dist(m_rand_gen) - 5; (see code above) and recompile, but that's clearly not optimal.

General info NASA Ames Stereo Pipeline 3.0.0 Build ID: 2d55403

Built against: NASA Vision Workbench 3.0.0 Build ID: 9048618 USGS ISIS 5.0.1 Boost C++ Libraries 106800 GDAL 2.4.1 | 20190315

oleg-alexandrov commented 2 years ago

This option was an experiment Scott put in, and he may have more to say on it, but I am not sure I can think of a situation where randomizing the camera positions is necessary. In satellite stereo the cameras are normally separated by several km, and wiggling each camera position by a handful of meters likely won't make much of a difference.

If you do want the cameras to go a bit wild, you can try the --input-adjustments-prefix option. You can cook up some .adjust files similar to those you get for the output for this tool, and let bundle adjustment start by applying those to the cameras. The .adjust format is not documented, and it is a bit hard to explain, but at least in principle, the three values on the first line are some shift in meters, and you can put any numbers there you want.

One can also try to experiment with --ip-per-tile and --camera-weight to give bundle adjustment more info to work with.

steo85it commented 2 years ago

Thanks for the reply!

wiggling each camera position by a handful of meters likely won't make much of a difference

This is precisely my issue with that option, too!

You can cook up some .adjust files similar to those you get for the output for this tool, and let bundle adjustment start by applying those to the cameras.

That's indeed what I did as a rough solution (setting up a loop with randomized km-level shifts in the form of --input-adjustments-prefix temporary files, and checking the convergence value to keep the minimum residuals among all tests), but it seemed a bit "house-cooked" and hard to book-keep.

In general, I'd say that checking convergence of bundle_adjust with different a priori cameras is a good idea to make the solution more robust. As you say, though, those initial guesses should be more widely spaced from each other to fulfill its purpose (I thought a few meters maybe were sufficient on the Moon, but I get that even there it's likely to be too little). That was indeed the goal of my "request", if you think this option should stay.

oleg-alexandrov commented 2 years ago

I will argue however that wiggling each camera even by 100 m won't help much either, if you want convergence, if your distance between cameras is 4000 m.

If you start with a physical configuration of cameras, whose positions and orientations are reasonably good, with a small uncertainty which needs to be minimized, which is much smaller than the baseline, there should be a unique solution.

If you start with a wild cooked up system of cameras, then likely random initializations won't help you. Especially that you randomize only the positions, and not orientations. It is orientations which are harder to get right, and when those are correct, the positions more or less fall into place in a unique way.

In short, while you have a valid point that our current random initialization logic is quite limited by the number 5 m being hard-coded, I would like to see a good case as to why allowing versatility in camera position initialization results in a good final solution. As it is, likely at some point that existing random 5 m perturbation logic could as well be wiped if it becomes work to maintain.

(Sorry for putting it that way, our resources here are rather limited, and there many things to be fixed, and sometimes one has to pick one's battles.)

On Mon, Oct 18, 2021 at 6:01 PM Stefano Bertone @.***> wrote:

Thanks for the reply!

wiggling each camera position by a handful of meters likely won't make much of a difference

This is precisely my issue with that option, too!

You can cook up some .adjust files similar to those you get for the output for this tool, and let bundle adjustment start by applying those to the cameras.

That's indeed what I did as a rough solution (setting up a loop with randomized shifts in the form of --input-adjustments-prefix temporary files, and checking the convergence value to keep the minimum residuals among all tests), but I seemed a bit "house-cooked" and hard to book-keep.

In general, I'd say that checking convergence of bundle_adjust with different a priori cameras is a good idea to make the solution more robust. As you say, though, those initial guesses should be more widely spaced from each other to fulfill its purpose (I thought a few meters maybe were sufficient on the Moon, but I get that even there it's likely to be too little). That was indeed the goal of my "request", if you think this option should stay.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/NeoGeographyToolkit/StereoPipeline/issues/344#issuecomment-946282081, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKDU3DXSR7RXKHPRHTIUTDUHS7PRANCNFSM5GHTPVAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

steo85it commented 2 years ago

That's all fair points and no problem, I perfectly understand (and greatly appreciate the tool your team put together and is developing and maintaining). I can live with my own "house-made" implementation for testing in those cases when bundle_adjust converges to weird solutions (and other likely causes have been excluded). I'll close this and let you know if I find (multiple) practical cases in which this helps significantly improve the converged solution.

oleg-alexandrov commented 2 years ago

I will be happy to hear from you when a case is actually encountered and playing with camera centers helps. My experience is that when bundle adjustment fails, one better take a closer look and see if something is strange with the configuration (interest points, intrinsic parameters, etc). If orientations are messed up, likely nothing short of structure-from-motion will help (for which we have camera_solve).

That is not to say that bundle adjustment has a unique solution. In fact, its significant weakness that there are infinitely many subtly different solutions, and alignment to ground followed by some ground constraints (which ASP supports in various ways) might be necessary to actually arrive at a reliable camera solution which can result in consistent terrain models.