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.

Sign up for GitHub

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

Jump to bottom

Log the sizes of each connected component in viewgraph #668

Closed
wants to merge 8 commits into from
Closed

Log the sizes of each connected component in viewgraph #668

wants to merge 8 commits into from

Conversation

johnwlambert
Copy link
Collaborator

No description provided.

travisdriver
travisdriver reviewed Jul 25, 2023
View reviewed changes
Copy link
Collaborator

@travisdriver travisdriver left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Could we just add this to a metric output file or something? The printout could be quite long depending on the dataset.

This information is also available more or less in the view graph image we already output.

@akshay-krishnan
Copy link
Collaborator

akshay-krishnan commented Jul 25, 2023 •
edited
Loading

yeah I agree with @travisdriver. it would be as simple as adding one scalar metric for each connected component..

@johnwlambert
Copy link
Collaborator Author

Could we just add this to a metric output file or something? The printout could be quite long depending on the dataset.

This information is also available more or less in the view graph image we already output.

Acknowledged. I've found this information helpful and unobtrusive, e.g.

Notre Dame DFE
2023-07-25T05:04:41.8002951Z [2023-07-25 05:04:41,799 INFO graph.py line 40 3264] Connected component sizes: [11] nodes.

Skydio-8
2023-07-25T05:03:09.0442467Z [2023-07-25 05:03:09,043 INFO graph.py line 40 3242] Connected component sizes: [8] nodes.

Skydio-32 deep front end
2023-07-25T05:06:33.0622333Z [2023-07-25 05:06:33,061 INFO graph.py line 40 3320] Connected component sizes: [32] nodes.

Astrovision sift front end
2023-07-25T05:12:22.6848402Z [2023-07-25 05:12:22,684 INFO graph.py line 40 3248] Connected component sizes: [47, 16] nodes.

Palace fine arts sift-281
2023-07-25T05:31:45.2373141Z [2023-07-25 05:31:45,236 INFO graph.py line 40 3313] Connected component sizes: [280] nodes.

Our main logging issue currently is that we log 47,000 lines of "CheiralityException: Landmark p5193 behind Camera x62" for palace-of-fine-arts.

@travisdriver
Copy link
Collaborator

Could we just add this to a metric output file or something? The printout could be quite long depending on the dataset.
This information is also available more or less in the view graph image we already output.

Acknowledged. I've found this information helpful and unobtrusive, e.g.

Notre Dame DFE
2023-07-25T05:04:41.8002951Z [2023-07-25 05:04:41,799 INFO graph.py line 40 3264] Connected component sizes: [11] nodes.

Skydio-8
2023-07-25T05:03:09.0442467Z [2023-07-25 05:03:09,043 INFO graph.py line 40 3242] Connected component sizes: [8] nodes.

Skydio-32 deep front end
2023-07-25T05:06:33.0622333Z [2023-07-25 05:06:33,061 INFO graph.py line 40 3320] Connected component sizes: [32] nodes.

Astrovision sift front end
2023-07-25T05:12:22.6848402Z [2023-07-25 05:12:22,684 INFO graph.py line 40 3248] Connected component sizes: [47, 16] nodes.

Palace fine arts sift-281
2023-07-25T05:31:45.2373141Z [2023-07-25 05:31:45,236 INFO graph.py line 40 3313] Connected component sizes: [280] nodes.

Our main logging issue currently is that we log 47,000 lines of "CheiralityException: Landmark p5193 behind Camera x62" for palace-of-fine-arts.

I feel as though this information isn't very useful. What would be more useful is putting this in some sort of file that actually lists the image numbers associated with each cluster.

akshay-krishnan
akshay-krishnan reviewed Jul 26, 2023
View reviewed changes
gtsfm/utils/graph.py
Comment on lines +43 to +44
largest_cc_nodes = max(nx.connected_components(input_graph), key=len)
subgraph = input_graph.subgraph(largest_cc_nodes).copy()
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you change this API to return the subgraph for every component? its potentially useful if we decide to optimize multiple connected components. And will also help you save those metrics..

akshay-krishnan
akshay-krishnan reviewed Oct 19, 2023
View reviewed changes
gtsfm/utils/graph.py
Comment on lines +39 to +43
cc_sizes = [len(x) for x in sorted(list(nx.connected_components(input_graph)))]
logger.info("Connected component sizes: %s nodes.", str(cc_sizes))

# Get the largest connected component.
largest_cc_nodes = max(nx.connected_components(input_graph), key=len)
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you actually calling nx.connected_components twice here? :0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@travisdriver travisdriver travisdriver left review comments

@akshay-krishnan akshay-krishnan akshay-krishnan left review comments

@ayushbaid ayushbaid ayushbaid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@johnwlambert @akshay-krishnan @travisdriver @ayushbaid