sinzlab / sensorium

Code base for the SENSORIUM competition.
https://sensorium2022.net/
MIT License
60 stars 32 forks source link

core_bias was ignored and set to wrong default value #116

Closed MaxFBurg closed 8 months ago

MaxFBurg commented 8 months ago

Because independent_bn_bias was per default True: https://github.com/ecker-lab/neuralpredictors/blob/7802bed1fda9b6536ff7bf94326fb45a50cfa1d3/neuralpredictors/layers/cores/conv2d.py#L55

the bias parameter was ignored: https://github.com/ecker-lab/neuralpredictors/blob/7802bed1fda9b6536ff7bf94326fb45a50cfa1d3/neuralpredictors/layers/cores/conv2d.py#L208-L225 . Overall, the default was to add scale and bias to the batch norm: https://github.com/ecker-lab/neuralpredictors/blob/7802bed1fda9b6536ff7bf94326fb45a50cfa1d3/neuralpredictors/layers/cores/conv2d.py#L211

We removed independent_bn_bias in https://github.com/sinzlab/neuralpredictors/pull/221 . The default behavior of the core remained unchanged (batch_norm_scale and bias default to true): https://github.com/ecker-lab/neuralpredictors/blob/e354212e227793ef3e2d465739854e0f154f01b5/neuralpredictors/layers/cores/conv2d.py#L61 and https://github.com/ecker-lab/neuralpredictors/blob/e354212e227793ef3e2d465739854e0f154f01b5/neuralpredictors/layers/cores/conv2d.py#L56

However, since the sensorium model sets core_bias=False per default - which was ignored and behaved like core_bias=True because independent_bn_bias=True - now core_bias=True this should be the new default to (a) make that argument work in the first place and (b) keep the same behavior as with the neural predictors version from back then.

KonstantinWilleke commented 8 months ago

Good catch - Thanks a lot!