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

fix: remove redundant final_activation layer in model.py #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JCasaraconn
Copy link

Remove redundant final_activation layer in the forward() method of AbstractUNet.

The layer is redundant because it is only applied during validation, however the loss function of segmentation models already applies a final activation layer. Therefore the final_activation layer is applied twice in these instances.

@JCasaraconn JCasaraconn changed the title feat: remove redundant final_activation layer in model.py fix: remove redundant final_activation layer in model.py Oct 4, 2024
Remove redundant final_activation layer in the forward() method
of AbstractUNet.

The layer is redundant because it is only applied during validation,
however the loss function of segmentation models already applies
a final activation layer. Therefore the final_activation layer
is applied twice in these instances.
@JCasaraconn JCasaraconn force-pushed the remove-redundant-final-activation branch from 249a4a4 to 649d109 Compare October 4, 2024 02:49
@JCasaraconn
Copy link
Author

I'm still thinking whether or not this means the validation eval should apply the final_activation layer, similar to the train eval as described here:

if isinstance(self.model, nn.DataParallel):
final_activation = self.model.module.final_activation
else:
final_activation = self.model.final_activation
if final_activation is not None:
act_output = final_activation(output)
else:
act_output = output
eval_score = self.eval_criterion(act_output, target)

I'm curious, what are your thoughts?

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.

1 participant