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/sg 849 add replace in channels #1557

Merged
merged 37 commits into from
Oct 29, 2023

Conversation

Louis-Dupont
Copy link
Contributor

@Louis-Dupont Louis-Dupont commented Oct 21, 2023

Adding replace_input_channels and get_input_channels to SgModel (and other required stems/backbones)

@Louis-Dupont Louis-Dupont marked this pull request as ready for review October 21, 2023 20:52
@shaydeci
Copy link
Collaborator

shaydeci commented Oct 23, 2023

Adding replace_input_channels and get_input_channels to SgModel (and other required stems/backbones)

Question

How to handle models that were implemented with self.in_channels attribute, since we now have get_input_channels() method that dynamically checks the value without relying on an attribute?

a. Should we remove any self.in_channels attribute?

pro: cleaner IMO con: risk of not being backward compatible if models are built on top of these models.

b. Or should we update the value of self.in_channels when calling replace_input_channels() ? pro: 100% backward compatible con: risk of bug if we miss one place where self.in_channels should be updated con: redundant and maybe a bit more confusing

Most important thing this PR is missin, is a mechanism to replace the stem out of the box when using pretrained weights.
One should be able to use pretrained checkpoints, replacing the stem behind the scenes.

@Louis-Dupont
Copy link
Contributor Author

Louis-Dupont commented Oct 24, 2023

Most important thing this PR is missin, is a mechanism to replace the stem out of the box when using pretrained weights.
One should be able to use pretrained checkpoints, replacing the stem behind the scenes.

Added here 287b28f

@Louis-Dupont
Copy link
Contributor Author

Added update of self.in_channels when calling replace_input_channels: ab760e8

@Louis-Dupont Louis-Dupont force-pushed the feature/SG-849-add_replace_in_channels branch from 44d9e93 to df11873 Compare October 29, 2023 13:24
Copy link
Collaborator

@shaydeci shaydeci left a comment

Choose a reason for hiding this comment

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

LGTM

@shaydeci shaydeci merged commit c0594a3 into master Oct 29, 2023
7 checks passed
@shaydeci shaydeci deleted the feature/SG-849-add_replace_in_channels branch October 29, 2023 17:00
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