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

Concern regarding parameter initialization in PoincareLinear module #2

Open
nbuton opened this issue Apr 4, 2023 · 1 comment
Open

Comments

@nbuton
Copy link

nbuton commented Apr 4, 2023

Dear Authors,

Thank you for your work on this project. I have a concern regarding the initialization of parameters in the PoincareLinear module, specifically with regard to the two parameters defined: one for the weight and the other for the norm of the weight.

It appears that the two parameters are defined independently, which could lead to inconsistencies when the optimizer changes the weight and weight norm separately. I believe that these two parameters should be connected in order to ensure that they are changed together. More precisely, only weight_v should be a parameter and weigh_g should be computed in the forward pass.

In order to validate my hypothesis, I recommend temporarily adding an assert statement in the forward pass as shown below:

assert torch.sum(torch.abs(self.weight_v.norm(dim=0)-self.weight_g))<0.1

This will not verify the assertion very quickly.

I could be wrong, but I would appreciate your clarification on this matter.

Thank you in advance.

@abfariah
Copy link

abfariah commented Oct 6, 2023

I very much agree with @nbuton, as I also was astonished by this parameter initialization. It would be great to hear from the authors.

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

No branches or pull requests

2 participants