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 Image masking #380

Merged
merged 21 commits into from
Dec 9, 2021
Merged

Add Image masking #380

merged 21 commits into from
Dec 9, 2021

Conversation

travisdriver
Copy link
Collaborator

Added an optional mask to the Image class, and also added functionality for filtering extracted keypoints with respect to this mask.

An example where this may be useful are when images are consistently obstructed (e.g., a spacecraft rendezvous with the ISS, landing on the Moon).

Moreover, from a previous project, it has also been shown that masking out dark regions (pixel value of ~0) of images captured during small body relative navigation scenarios (e.g., the interface between the foreground body and the background deep space, self-shadowing) substantially increases the reliability of the visual measurements:

No masking

Screenshot from 2021-11-16 00-25-16

Masking

Screenshot from 2021-11-16 00-23-12

@travisdriver travisdriver requested review from johnwlambert and akshay-krishnan and removed request for johnwlambert November 16, 2021 15:24
gtsfm/utils/images.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@akshay-krishnan akshay-krishnan left a comment

Choose a reason for hiding this comment

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

I think John is addressing most concerns I had, I am also not very familiar with the frontend APIs. I can take a final look once all other comments are addressed, if necessary.

@johnwlambert
Copy link
Collaborator

@travisdriver thanks for the updates. Can you add the masked and unmasked jobs to the CI like we discussed in our last meeting? That way we can track improvements even without the mask, and we can ensure we retain good performance on the masked job without degradation

@travisdriver
Copy link
Collaborator Author

@travisdriver thanks for the updates. Can you add the masked and unmasked jobs to the CI like we discussed in our last meeting? That way we can track improvements even without the mask, and we can ensure we retain good performance on the masked job without degradation

I'd like to save that for a different PR if possible, as I want to change a couple things about the AstroNet benchmarks.

@johnwlambert
Copy link
Collaborator

@travisdriver thanks for the updates. Can you add the masked and unmasked jobs to the CI like we discussed in our last meeting? That way we can track improvements even without the mask, and we can ensure we retain good performance on the masked job without degradation

I'd like to save that for a different PR if possible, as I want to change a couple things about the AstroNet benchmarks.

@travisdriver Sure thing, can you open an issue so we don't lose track of it later?

Could you also upload the visual dashboard file here in the comments, so we can see the diff?

@johnwlambert
Copy link
Collaborator

@travisdriver thanks for the updates. Can you add the masked and unmasked jobs to the CI like we discussed in our last meeting? That way we can track improvements even without the mask, and we can ensure we retain good performance on the masked job without degradation

I'd like to save that for a different PR if possible, as I want to change a couple things about the AstroNet benchmarks.

@travisdriver Sure thing, can you open an issue so we don't lose track of it later?

Could you also upload the visual dashboard file here in the comments, so we can see the diff?

Hi @travisdriver, I think this PR is almost ready to merge : - )
Just these last two things here above

@@ -99,18 +99,32 @@ def get_top_k(self, k: int) -> "Keypoints":
subset of current keypoints.
"""
if k >= len(self):
return copy.deepcopy(self)
return copy.deepcopy(self), np.arange(self.__len__())
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: len(self) looks better. I am fine with this too.

@johnwlambert
Copy link
Collaborator

dashboard:
visual_comparison_dashboard.html 2.zip

@johnwlambert johnwlambert merged commit af4997a into master Dec 9, 2021
@johnwlambert johnwlambert deleted the image-masks branch December 9, 2021 15:18
@travisdriver travisdriver linked an issue Dec 10, 2021 that may be closed by this pull request
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.

Resize image no op
4 participants