OpenSfM icon indicating copy to clipboard operation
OpenSfM copied to clipboard

Allow reconstructions to be extended

Open BrookRoberts opened this issue 8 years ago • 10 comments

Not necessarily final, but would like to show the direction I've gone with this before going any further, so looking for comments @paulinus

These changes as a whole should allow you to run opensfm to generate a reconstruction, then place a few new images in the images folder, and then run the opensfm commands again - it will add the new images to the reconstruction without recalculating everything.

Mostly we just check if things are already calculated (e.g. features) and don't redo. For matches, we only calculate matches for the new images, and save them all in the folders of the new images (previously they would go in the image that had the first name alphabetically). For create_tracks we save the unionfind structure, as well as track_ids, which allows us to use previous reconstructions with updated tracks, since we have a persistent way of naming the tracks.

From a test on a large dataset: First run generating reconstruction of 922 images: focal_from_exif: 13.5280120373 detect_features: 344.613101006 match_features: 4669.25260997 create_tracks: 129.082221985 reconstruct: 4923.34434319 mesh: 77.7916591167

Re run with no extra images: focal_from_exif: 0.0260539054871 detect_features: 0.034029006958 match_features: 0.200579881668 create_tracks: 97.3890349865 reconstruct: 188.215790033 mesh: 79.0046401024

Re run with 15 extra images: focal_from_exif: 0.240912914276 detect_features: 8.93757390976 match_features: 121.738699913 create_tracks: 104.183819056 reconstruct: 442.458048105 mesh: 81.0022370815

(incidentially, this was done on an old set I had used, and it looked like it ran a lot faster than last time - has opensfm had any recent changes that make it faster? or did something change on my end?)

There are still some bits that will be slower with larger reconstructions.

  1. Some of these are because I just haven't taken away the calculations yet: In create_tracks, we convert the unionfind into a graph everytime still.
  2. Some of these are loading times - in adding to an existing reconstruction, we load large files into memory. There might be clever things here that could be done, but there might not.
  3. I haven't changed the mesh command - it might be possible to do something here, or maybe you do have to recalculate whole thing after changing the reconstruction - I haven't looked.
  4. Reconstruction bundle step is slower with larger datasets, this is a general problem.

Problems: It might result in unexpected behaviour, if you change the images/config, and it doesn't take this into account but uses the already saved results. detect_features already did this, so this change may be an improvement as it makes it more obvious that stuff is not being recalculated (and that you should use clean command to do a new build).

Thoughts? If the approach looks vaguely sensible (the way I'm saving things in create_tracks is maybe not very nice at the moment), and you'd be happy for this to go into master, I can tidy things up/make high level improvements. I'd then appreciate someone thinking about the changes/asking questions reasonably carefully to make sure things work as I think they do.

BrookRoberts avatar May 24 '17 10:05 BrookRoberts

#136 is related to this

BrookRoberts avatar May 24 '17 11:05 BrookRoberts

That is really great @BrookRoberts ! i need some time to parse it.

The tricky part is updating the tracks graph. I wonder if it is possible to do without storing the unionfind structure. The information is already in the tracks.csv file although not in a different form. I'm thinking that the structure can be recreated by linking features that belong to the same track, but it seems hard to keep the same track ids.

paulinus avatar May 30 '17 15:05 paulinus

No problem - I'm happy to answer any questions if anything isn't obvious.

Yes, saving the unionfind structure is a bit irritating. It is always going to be small in size compared to the size of the images I think, so I don't think it will add much space/time issues, but it's yucky from a clean code point of view. It seemed to me that it was likely to be less efficient to convent tracks.csv back to unionfind than just saving and loading.

I don't have a good idea of how efficient the unionfind structure is, but I've currently just assumed that the two step of creating unionfind then converting to tracks is good for efficiency.

BrookRoberts avatar May 30 '17 15:05 BrookRoberts

@paulinus what is the status on merging this in? Are you intending to/thinking about how to do it without storing the unionfind structure? Did I misunderstand and you are waiting on me to make some change to that part?

I ask because it no longer merges in cleanly, due to refactoring in match_features.py. I can fix up the merge conflicts, but don't want to have to maintain a branch and keep fixing conflicts! (and this makes it more likely that my changes won't be so fully tested/will have bugs)

BrookRoberts avatar Jun 21 '17 09:06 BrookRoberts

Given that this isn't actually a trivial merge, but e.g. in match_features has changed in a way that assumes a bit that everything is being recalculated, can we get this merged in before other changes to these files go in?

If you strongly wanted, we could merge the parts that don't involve unionfind first, since each step is kind of standalone, but it would be a lot easier if all the parts went in. I'm not really willing to rewrite stuff every time something gets refactored/restructured.

BrookRoberts avatar Jun 21 '17 10:06 BrookRoberts

@paulinus - are you interested in this ever being merged in?

BrookRoberts avatar Aug 24 '17 11:08 BrookRoberts

@BrookRoberts does this branch currently work out of the box with the latest version of OpenSFM? It would be really useful for a project I'm working on, thanks!

mluogh avatar Sep 16 '17 04:09 mluogh

@deltameter - sorry for slow reply. To my knowledge, it does, but can't guarantee it is heavily tested. Would be interested in knowing if it works for you / what issues it has. As above, it does still recalculate somethings it might not need to, but is substantially better than not using it.

BrookRoberts avatar Sep 27 '17 10:09 BrookRoberts

Hello @BrookRoberts I have a reconstruction of Spherical images and I want to add one Pinhole image(same scene captured) to the scene. Do you think that can be achieved?

happyfrank avatar Feb 25 '18 10:02 happyfrank

Hi @BrookRoberts!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot avatar Jul 17 '20 07:07 facebook-github-bot