Allow reconstructions to be extended #178
Conversation
BrookRoberts
changed the title
|
#136 is related to this |
|
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 |
|
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. |
|
@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) |
|
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
force-pushed
the
create_tracks_improvement
branch
from
fca7cc6
to
84bcad1
Compare
|
@paulinus - are you interested in this ever being merged in? |
|
@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! |
|
@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. |
|
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? |
|
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! |
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.
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.