Skip to content
This repository was archived by the owner on Oct 9, 2023. It is now read-only.

Remove embedding_dim parameter in ImageEmbedder examples #655

Conversation

tszumowski
Copy link
Contributor

What does this PR do?

What

  • Removed embedding_dim parameter in the ImageEmbedder README example
  • Removed embedding_dim parameter in the ImageEmbedder code example
  • Searched for all other references to embedding_dim usgae -- Only other one is in a unit test, which should stay

Why

Fixes # (issue)

Before submitting

PR review

  • Is this pull request ready for review? (if not, please submit in draft mode)

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@tszumowski
Copy link
Contributor Author

Uh oh. I now realize that you may need to flatten that embedding for that resnet example. Because IIRC, resnet returns a 2048x7x7 output. Will put into draft mode for now, and take it out of draft when ready with a test result.

@tszumowski
Copy link
Contributor Author

I found a potential error. If you run without embedding_dim assigned, it takes the last layer of the backbone. That's not always flattened. For example, in Resnet101, it's 2048x7x7. That's not compatible with embedding applications without flattening.

I'll close this PR for now and discuss this in a separate issue.

@tszumowski
Copy link
Contributor Author

Discussion migrated to:
#656

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant