OpenSfM icon indicating copy to clipboard operation
OpenSfM copied to clipboard

pair_selections drops all gps_neighbors, graph_rounds to 0

Open disarticulate opened this issue 6 months ago • 3 comments

code:

    #[...] /opensfm/pairs_selection.py
    if not all(map(has_gps_info, exifs.values())):
        if gps_neighbors != 0:
            logger.warn(
                "Not all images have GPS info. " "Disabling matching_gps_neighbors."
            )
        gps_neighbors = 0
        max_distance = 0
        graph_rounds = 0

Matching all photos is a aggressive drop down in this situation. My current issue is our drone screws up the first image in the process, but otherwise logs all GPS data to the exif. I'm also providing a geo.txt file in ODM. Am unsure if this process cares about the geo.txt file, but regardless, it seems like a improve default when certain gps files do not have gps would be segregate them.

The rest of the function is confusing. It looks like images_ref and cand_images and is only ever called with images from:

def run_dataset(data: DataSetBase) -> None:
    """Match features between image pairs."""

    images = data.images()

    start = timer()
    pairs_matches, preport = matching.match_images(data, {}, images, images)
    matching.save_matches(data, images, pairs_matches)
    matching.clear_cache()
    end = timer()
    write_report(data, preport, list(pairs_matches.keys()), end - start)

then exifs is:

    # Get EXIFs data
    all_images = list(set(ref_images + cand_images))
    exifs = {im: data.load_exif(im) for im in all_images}

so it seems like we could segregate images_ref and images_cand into a has_gps bucket before the all(*) check:

    exif_items = exifs.items()
    exif_found_gps = map(has_gps_info, [v for k,v in exif_items])
    images_ref_has_gps = []
    images_cand_has_gps = []
    for i, has_gps in exif_found_gps:
        if has_gps:
            images_ref_has_gps.append(images_ref[i])
            images_cand_has_gps.append(images_ref[i])

Note, I don't think the order is garunteed, so another function would need to find it, or it would need to be garunteed by caller.

I'm not sure how the rest of the logic goes, but we're simply trying to figure out pairs to run matching on. It seems like triggering the all pair selection:

    if (
        max_distance
        == gps_neighbors
        == time_neighbors
        == order_neighbors
        == bow_neighbors
        == vlad_neighbors
        == graph_rounds
        == 0
    ):
        # All pair selection strategies deactivated so we match all pairs
        d = set()
        t = set()
        g = set()
        o = set()
        b = set()
        v = set()
        pairs = {sorted_pair(i, j) for i in images_ref for j in images_cand if i != j}

Kills processing time with just one image missing

disarticulate avatar Jul 27 '25 19:07 disarticulate

I deleted this image from the dataset and now the matcher is finding mostly true matches. There may be a bug elsewhere as even when order_neighbors is set in ODM via matcher-order, defaults to 0 when georeference reconstruction is found:

#osfm.py
            if args.matcher_order > 0:
                if not reconstruction.is_georeferenced():
                    config.append("matching_order_neighbors: %s" % args.matcher_order)
                else:
                    log.ODM_WARNING("Georeferenced reconstruction, ignoring --matcher-order")

This triggers opensfm to degrade to worse case pairs matching. Not sure how to resolve this contention. As far as I understand OpenSfM should be able to operate in a mixed gps reference or on images + gcp.

disarticulate avatar Jul 27 '25 20:07 disarticulate

It also looks like a regression because of this commit: https://github.com/OpenDroneMap/ODM/commit/6f6827091f184664babbc8aa119d28a2bab43645

Where matcher_graph_rounds would not trigger the worse case degrade, but now does. It seems like the correct route would be to segregate candidates for matching between has gps and not when doing the relevant distance comparison. I understand the conceptual pairs logic, but not the various logical consistencies nor the types of the lists. A little direction, and I could probably prepare a patch.

disarticulate avatar Jul 27 '25 20:07 disarticulate

If someone wants to see the dataset, it's here: https://community.opendronemap.org/t/kandiyohi-county-landfill/25254

disarticulate avatar Aug 25 '25 20:08 disarticulate