-
Notifications
You must be signed in to change notification settings - Fork 24
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 similarity high-level interface #107
Conversation
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
==========================================
- Coverage 99.95% 99.87% -0.09%
==========================================
Files 51 57 +6
Lines 2074 2327 +253
==========================================
+ Hits 2073 2324 +251
- Misses 1 3 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
110d401
to
f7b7ec6
Compare
be4f29d
to
d76460e
Compare
be4f29d
to
3646e9e
Compare
Rebased to new master and addressed comments in fixup commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for getting this in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM. I made a few minor suggestions, and wanted to clarify whether we do actually want to remove the first dimension of the similarity-based saliency maps, as it makes it less consistent with our other saliency map outputs, which are generally of the form [N x H x W].
This saliency implementation transforms proximity in feature space into | ||
saliency heatmaps. | ||
This should require a sequence of feature vectors of the query and reference | ||
image, a number of feature vectors as predicted on perturbed version of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
image, a number of feature vectors as predicted on perturbed version of the | |
image, a number of feature vectors as predicted on perturbed versions of the |
:param ref_descr_2: | ||
Second image reference float feature-vector, shape `[nFeats]` | ||
:param ref_descr: | ||
Reference iamge float feature-vector, shape `[nFeats]` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference iamge float feature-vector, shape `[nFeats]` | |
Reference image float feature-vector, shape `[nFeats]` |
:param ref_descr: | ||
Reference iamge float feature-vector, shape `[nFeats]` | ||
:param query_descr: | ||
Query image reference float feature-vector, shape `[nFeats]`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Query image reference float feature-vector, shape `[nFeats]`. | |
Query image float feature-vector, shape `[nFeats]`. |
|
||
The perturbation masks used by the following implementation are | ||
expected to be of type integer. Masks containing values of type | ||
float are rounded to the nearest value and binarized | ||
with value 1 replacing values greater than or equal to half of | ||
the maximum value in mask after rounding while 0 replaces the rest. | ||
|
||
The resulting saliency maps are relative to the query image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the wording here is a bit ambiguous, I suggest adding the same text you used below for the interface:
The resulting saliency maps are relative to the query image. | |
The resulting saliency map is relative to the query image. | |
As such, it denotes regions in the query image that make it more or less | |
similar to the other image, called the reference image. |
"show_sal_map(test_image2, test_image1, sal_map)\n", | ||
"\n", | ||
"# a -> a\n", | ||
"sal_map = sal_generator(\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The saliency map ranges here for the last two image similarity saliency pairs are slightly different than in master, e.g. [0.0397, 1] vs. [0.0607, 1] and [0.044, 1] vs. [0.079, 1]. Do we know why this is? The shape of the saliency maps look generally consistent though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is due to the updated sliding window implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had environment issues and was running with the old sliding window implementation. Re-ran and it matches master now.
Errors:
|
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Highlights:
GenerateImageSimilarityBlackboxSaliency
PerturbationOcclusion
(following pattern of other interfaces) andSBSMStack
SimilarityScoring
saliency generation impl to return a [HxW] heatmap instead of [1xHxW]. This is consistent with the description of the interface that it implements.SBSMStack
. I got rid of the application style used by the previous version because the new interface takes care of all the logic performed by the "app". The previous version also mentions that it shows the use of multiple API implementations, but it only uses one set of perturbation/scoring implementations. I got rid of these mentions.