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

Feature/clip #246

Merged
merged 20 commits into from
Jan 21, 2022
Merged

Feature/clip #246

merged 20 commits into from
Jan 21, 2022

Conversation

lucaro
Copy link
Member

@lucaro lucaro commented Jan 13, 2022

  • Added Image and Text features based on OpenAI CLIP
  • refactored and reorganized some common helper logic

Copy link
Member

@Spiess Spiess left a comment

Choose a reason for hiding this comment

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

Looks great, thank you very much for the work.

I added two comments about System.outs that may have been left in from debugging. Once they are either removed or I have your confirmation that they exist intentionally I will be happy to give my approval.

@lucaro lucaro requested a review from Spiess January 20, 2022 14:53
Spiess
Spiess previously approved these changes Jan 20, 2022
Copy link
Member

@Spiess Spiess left a comment

Choose a reason for hiding this comment

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

Looks great, thanks again!

@silvanheller
Copy link
Member

Could this target master directly? I don't see a reason this has to target dev first

@lucaro
Copy link
Member Author

lucaro commented Jan 21, 2022

Could this target master directly? I don't see a reason this has to target dev first

I guess it could, I don't really have a preference for the target. If we want to make a 'minor release' out of this, sure.

@silvanheller silvanheller changed the base branch from dev to master January 21, 2022 15:28
@silvanheller silvanheller dismissed Spiess’s stale review January 21, 2022 15:28

The base branch was changed.

Copy link
Member

@silvanheller silvanheller left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the refactoring work!

@silvanheller silvanheller merged commit c2315f0 into master Jan 21, 2022
@silvanheller silvanheller deleted the feature/clip branch January 21, 2022 15:49
silvanheller added a commit that referenced this pull request Jun 10, 2022
* Added feature modules for OpenAI CLIP

* Minor refactoring of utility classes

* Refactored common image preprocessing logic into helper class

Co-authored-by: Silvan Heller <silvan.heller@unibas.ch>
Co-authored-by: Florian Spiess <florian.spiess@unibas.ch>
Former-commit-id: c2315f0
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.

None yet

3 participants