Open D1rk123 opened 1 month ago
Totally agree!
I think this is caused by LION still being a bit WIP and sometimes stuff gets changed because I test this or that paper etc.
Admittedly the current LPD is the one I found that works better than the boilerplate one, but recently we indeed decided that all default settings should follow the original paper, so we should 100% change this, and make the current default be something like LION_default_parameters
(or possibly a neater name) like we have for the alternatives: https://github.com/CambridgeCIA/LION/blob/a28be484022628c2b739b05080cdd113315ed675/LION/models/iterative_unrolled/LPD.py#L191-L215
Thanks for your quick reply!
Having a separate original paper settings, and LION default settings sounds like a good idea to me. In my (somewhat limited) experiments, LPD also performed better without convolution biasses. In the pull request I didn't include the two versions of the settings yet.
Yup, will likely do that. I hope to tidy all this soon and get some examples and demos by the end of the year, so it gets easier to use. Let me know if you have any questions meanwhile, as its all a bit in the middle of construction now
On Thu, 17 Oct 2024, 09:12 Dirk Schut, @.***> wrote:
Thanks for your quick reply!
Having a separate original paper settings, and LION default settings sounds like a good idea to me. In my (somewhat limited) experiments, LPD also performed better without convolution biasses. In the pull request I didn't include the two versions of the settings yet.
— Reply to this email directly, view it on GitHub https://github.com/CambridgeCIA/LION/issues/139#issuecomment-2418861871, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2OENHRYQ2WINNELNM5XSTZ35WNPAVCNFSM6AAAAABQBNZE6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJYHA3DCOBXGE . You are receiving this because you commented.Message ID: @.***>
In the LPD implementation the bias terms of the convolution layers are turned off (bias=False in nn.Conv2d). However, in the original LPD paper they say that the convolution layers do have biasses. (Section IV.A In the equation after the line "where the k:th component is given by"). I also checked the original code, and it does not specify the bias, but the Tensorflow default is True.
Was it a deliberate choice to not follow the paper? Otherwise I think it's better to change the code so that bias=True for all convolution layers.