Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Generalize SSL functionality to work on other datasets #555

Merged
merged 25 commits into from
Sep 15, 2021
Merged

Generalize SSL functionality to work on other datasets #555

merged 25 commits into from
Sep 15, 2021

Conversation

vale-salvatelli
Copy link
Contributor

@vale-salvatelli vale-salvatelli commented Aug 24, 2021 •
edited
Loading

This PR contains a few changes needed to make the SSLContainer compatible with new datasets and allow a user to run by simply creating a new augmentation config, no other changes needed to the repo, namely:

  • _get_transforms has been changed to accept new datasets without the need to touch the class
  • get_cxr_ssl_transform has been changed to avoid the hidden channel expansion and make that optional. It has been also renamed to get_ssl_transform because it has nothing specific to cxr
  • drop_last is now set as parameter of the InnerEyeVisionDataModule and the SSLContainer - that means it can be changed when initializing a new SSLContainer
  • documentation about bringing your own SSL model has been updated

@vale-salvatelli vale-salvatelli requested review from javier-alvarez and removed request for melanibe August 25, 2021 08:44
@vale-salvatelli vale-salvatelli changed the title WIP: Generalize SSL functionality to work on other datasets Generalize SSL functionality to work on other datasets Sep 14, 2021
@vale-salvatelli vale-salvatelli enabled auto-merge (squash) September 14, 2021 14:40
ant0nsc
ant0nsc approved these changes Sep 15, 2021
View reviewed changes
Copy link
Contributor

@ant0nsc ant0nsc left a comment

Choose a reason for hiding this comment

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

Looking good, just a few minor things

InnerEye/ML/SSL/lightning_containers/ssl_container.py Show resolved Hide resolved
InnerEye/ML/SSL/lightning_containers/ssl_container.py Show resolved Hide resolved
@vale-salvatelli vale-salvatelli merged commit 521c004 into main Sep 15, 2021
@vale-salvatelli vale-salvatelli deleted the vsalva/generalize_ssl branch September 15, 2021 10:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@javier-alvarez javier-alvarez javier-alvarez left review comments

@ant0nsc ant0nsc ant0nsc approved these changes

@melanibe melanibe melanibe approved these changes

@Shruthi42 Shruthi42 Awaiting requested review from Shruthi42

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@vale-salvatelli @javier-alvarez @ant0nsc @melanibe