EHN Normalizing the last layer in all the models

[brunaafl]

Changed last layers' names to final_layer, cf #457.

Fix TIDNet HybridNet's and inheritance (open issue).

[sliwy]

@brunaafl You may want to take a look at #513 as there may be some conflicts and try to coordinate/see what are the common parts :)

[brunaafl]

@brunaafl You may want to take a look at #513 as there may be some conflicts and try to coordinate/see what are the common parts :)

Thanks! I will!

[codecov]

Codecov Report

Merging #520 (41d8ee0) into master (64b8e38) will increase coverage by 0.16%.
Report is 2 commits behind head on master.
The diff coverage is 93.91%.

@@            Coverage Diff             @@
##           master     #520      +/-   ##
==========================================
+ Coverage   84.55%   84.72%   +0.16%     
==========================================
  Files          63       63              
  Lines        4676     4741      +65     
==========================================
+ Hits         3954     4017      +63     
- Misses        722      724       +2     

[brunaafl]

TODO: Add a new way to handle return features

During the renaming, I had to remove the possibility of returning the output of the feature extractor when there was this option (on the SleepStagerEldele2021, SleepStagerBlanco2020, SleepStagerChambon2018, and DeepSleepNet models). However, this is required for some tutorials, so now they are broken.

@PierreGtch

[PierreGtch]

@brunaafl on it

[PierreGtch]

can you update the whats_new?

[brunaafl]

can you update the whats_new?

Yes! I'll do that

[sliwy]

thx for the PR @brunaafl

We discussed with @PierreGtch more in details how it should be done to support the transfer learning approach and be easy to use. We identified few cases in which we may act in slightly modified manner, so the PR will require some changes. We would be really happy if you find some time and try to address the changes:

1. The case in which we have nn.Module -> we would like to have the last layer providing the tensor with on of the dimensions equal to n_outputs to be called final_layer and all the modules after should be also incorporated into the nn.Sequential with it. Sometimes it requires splitting nn.Sequential into two parts or merging nn.Linear with some reshaping. Reshaping should be included in the final_layer because if we replace final_layer with nn.Identity then it will break the code because of mismatch of dimensions.
2. If the model is nn.Sequential we would like to have the last layer providing the tensor with on of the dimensions equal to n_outputs to be called final_layer and all the modules after incorporated into the same module.
3. LogSoftmax -> we would like to not touch LogsSoftmax as it is going to be renamed in another PR and it's going to solve a different problem. Let's assume that LogSoftmax is not changing the shape of the tensor and it should not be included in the final_layer.

@brunaafl would you have time to tackle that? if you have any doubts I would be happy to help! 😄

[brunaafl]

thx for the PR @brunaafl

We discussed with @PierreGtch more in details how it should be done to support the transfer learning approach and be easy to use. We identified few cases in which we may act in slightly modified manner, so the PR will require some changes. We would be really happy if you find some time and try to address the changes:

1. The case in which we have nn.Module -> we would like to have the last layer providing the tensor with on of the dimensions equal to n_outputs to be called final_layer and all the modules after should be also incorporated into the nn.Sequential with it. Sometimes it requires splitting nn.Sequential into two parts or merging nn.Linear with some reshaping. Reshaping should be included in the final_layer because if we replace final_layer with nn.Identity then it will break the code because of mismatch of dimensions.
2. If the model is nn.Sequential we would like to have the last layer providing the tensor with on of the dimensions equal to n_outputs to be called final_layer and all the modules after incorporated into the same module.
3. LogSoftmax -> we would like to not touch LogsSoftmax as it is going to be renamed in another PR and it's going to solve a different problem. Let's assume that LogSoftmax is not changing the shape of the tensor and it should not be included in the final_layer.

@brunaafl would you have time to tackle that? if you have any doubts I would be happy to help! 😄

Hi! Yes, I can address those changes, although not today, just tomorrow and after. I'm happy to help!

[brunaafl]

thx for the PR @brunaafl
We discussed with @PierreGtch more in details how it should be done to support the transfer learning approach and be easy to use. We identified few cases in which we may act in slightly modified manner, so the PR will require some changes. We would be really happy if you find some time and try to address the changes:

1. The case in which we have nn.Module -> we would like to have the last layer providing the tensor with on of the dimensions equal to n_outputs to be called final_layer and all the modules after should be also incorporated into the nn.Sequential with it. Sometimes it requires splitting nn.Sequential into two parts or merging nn.Linear with some reshaping. Reshaping should be included in the final_layer because if we replace final_layer with nn.Identity then it will break the code because of mismatch of dimensions.
2. If the model is nn.Sequential we would like to have the last layer providing the tensor with on of the dimensions equal to n_outputs to be called final_layer and all the modules after incorporated into the same module.
3. LogSoftmax -> we would like to not touch LogsSoftmax as it is going to be renamed in another PR and it's going to solve a different problem. Let's assume that LogSoftmax is not changing the shape of the tensor and it should not be included in the final_layer.

@brunaafl would you have time to tackle that? if you have any doubts I would be happy to help! 😄

HI! I was wondering here, just to know if I really understood. So, you want the first layer that changes the shape of the tensor to one with one of the dimensions equal to n_outputs to be called final_layer, and all the other modules after this one incorporated into the same sequential with it. Is that right? And the LogSoftmax should not be included in this.

My doubt is in the case where LogSoftmax comes in between a conv that changes the shape of one of the dimensions to n_outputs and another layer (such as one to squeeze the final output), such as in the case of Deep4Net and EEGNet. Should I just leave LogSoftmax inside the final_layer module, since it will be changed in a next pr?

[PierreGtch]

@brunaafl sorry for the confusion, there was some miscommunication at multiple levels.

So everything after and including the classification layer (sometimes it is ann.Linear, sometimes nn.Conv2d..) must go into this new final_layer.
Typically you will find a softmax, a reshape, a transpose, or a squeeze, etc. They should all go inside final_layer. For example, Deep4net will change from:

        self.add_module(
            "conv_classifier",
            nn.Conv2d(
                self.n_filters_4,
                self.n_outputs,
                (self.final_conv_length, 1),
                bias=True,
             ),
        )
        self.add_module("softmax", nn.LogSoftmax(dim=1))
        self.add_module("squeeze", Expression(squeeze_final_output))

TO:

        self.add_module("final_layer", nn.Sequential())
        self.final_layer.add_module(
            "conv_classifier",
            nn.Conv2d(
                self.n_filters_4,
                self.n_outputs,
                (self.final_conv_length, 1),
                bias=True,
             ),
        )
        self.final_layer.add_module("softmax", nn.LogSoftmax(dim=1))
        self.final_layer.add_module("squeeze", Expression(squeeze_final_output))

If it's still not clear, please tell me

[brunaafl]

@brunaafl sorry for the confusion, there was some miscommunication at multiple levels.

So everything after and including the classification layer (sometimes it is ann.Linear, sometimes nn.Conv2d..) must go into this new final_layer. Typically you will find a softmax, a reshape, a transpose, or a squeeze, etc. They should all go inside final_layer. For example, Deep4net will change from:

        self.add_module(
            "conv_classifier",
            nn.Conv2d(
                self.n_filters_4,
                self.n_outputs,
                (self.final_conv_length, 1),
                bias=True,
             ),
        )
        self.add_module("softmax", nn.LogSoftmax(dim=1))
        self.add_module("squeeze", Expression(squeeze_final_output))

TO:

        self.add_module("final_layer", nn.Sequential())
        self.final_layer.add_module(
            "conv_classifier",
            nn.Conv2d(
                self.n_filters_4,
                self.n_outputs,
                (self.final_conv_length, 1),
                bias=True,
             ),
        )
        self.final_layer.add_module("softmax", nn.LogSoftmax(dim=1))
        self.final_layer.add_module("squeeze", Expression(squeeze_final_output))

If it's still not clear, please tell me

Okay, it's clear now, thanks!

[sliwy]

Thanks @brunaafl for new changes! :)

I added few more comments. Most important, in eegconformer.py there are 4 " in two docstrings, so we have to fix that before merge (I have added suggestions). Then, few details with naming attributes or removing old attributes.

@bruAristimunha do you have any idea why tests didn't run on this PR after last commit?

done

[bruAristimunha]

Thank you @brunaafl, @sliwy and @PierreGtch