Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tanks and temples dataset loader #653

Merged
merged 157 commits into from
Dec 3, 2023
Merged

Conversation

johnwlambert
Copy link
Collaborator

@johnwlambert johnwlambert commented Jul 4, 2023

Add image and mesh loader for scenes from the Tanks & Temple dataset.

gtsfm/configs/synthetic_front_end.yaml Outdated Show resolved Hide resolved

retriever:
_target_: gtsfm.retriever.sequential_retriever.SequentialRetriever
max_frame_lookahead: 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is just 2 enough?

open3d_mesh_path = tempfile.NamedTemporaryFile(suffix='.obj').name
open3d.io.write_triangle_mesh(filename=open3d_mesh_path, mesh=mesh)

loader_future = client.scatter(loader, broadcast=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just avoid futures for this file I guess?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm wouldn't we lose all parallelization benefits then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but since we are just loading from disk, we would not lose much in terms of time I think

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if we can just generate the matches apriori and use something like a cache to just load the matches from the disk.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also thinking this would be the best approach, as this is more for debugging and will likely only be used by us.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see -- if we wish to use the Tanks and Temples loader to get GT poses to measure pose error, what's the difference between computing matches offline vs. online?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the synthetic matching frontend is only used for debugging, and I'm not sure we should merge it into the main repo

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayushbaid @travisdriver wanted to revisit this -- I would prefer to keep as-is and make this code self-contained so that we only need to run one command, instead of having to make two new scripts (one to generate all the correspondences beforehand, and then making a new one to accept saved correspondences)

min_score: 0.2
max_frame_lookahead: 3

# retriever:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, thanks.

_target_: gtsfm.frontend.correspondence_generator.synthetic_correspondence_generator.SyntheticCorrespondenceGenerator
#dataset_root: /Users/johnlambert/Downloads/Tanks_and_Temples_Barn_410
#dataset_root: /usr/local/gtsfm-data/Tanks_and_Temples_Barn_410
dataset_root: /home/runner/work/gtsfm/gtsfm/Tanks_and_Temples_Barn_410 # Path for CI.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer this being loaded from the dataset root passed from the terminal, but its not an easy fix. So we should be fine with it.

open3d_mesh_path = tempfile.NamedTemporaryFile(suffix='.obj').name
open3d.io.write_triangle_mesh(filename=open3d_mesh_path, mesh=mesh)

loader_future = client.scatter(loader, broadcast=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but since we are just loading from disk, we would not lose much in terms of time I think

@johnwlambert johnwlambert merged commit 3588817 into master Dec 3, 2023
24 checks passed
@johnwlambert johnwlambert deleted the tanks-and-temples-loader branch December 3, 2023 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants