HazyResearch / hippo-code

Apache License 2.0
168 stars 30 forks source link

Possible Bug in model/unroll.py? #5

Closed chifenggengmengyue closed 3 years ago

chifenggengmengyue commented 3 years ago

Hi, I am trying to running the main process in model/hippo.py and the result of 'y=hippolegs(x)' in L#114 are all zeros, which confuses me as I thought should be the coefficient of LegS basis. I trace the problem back to L#301 in model/unroll.py and find s = op(A, s) involves only A and s (s is a 0 matrix), and the u is ignored. Is this a bug or it just works that way?

Also, thanks for the awsome paper&appendix, I learned a lot from that :)

albertfgu commented 3 years ago

Hi, thanks for pointing this out! Funnily enough, I discovered this bug a few days ago in my internal repo but forgot to propagate the change here. This line should be the fix: https://github.com/HazyResearch/hippo-code/blob/6ad6580bfdc3e9cb3c50950d430b9800ba68e8e1/model/unroll.py#L302 See if that fixes your issue.

Glad you liked the paper :) I will hopefully be updating this repo in the coming weeks with an improved version, stay tuned...

chifenggengmengyue commented 3 years ago

Great, it works now, appreciate the quick response! looking forward to the improved version, star & watched.

Edit: I found the reconstruct method in hippo.py, looks like I used incorrect t values...Thanks again. -----------------------------------Old Question---------------------------- May I ask one more question? Is the computed 'y[t,..,:]' exactly the coefficients c(t) in your paper? I am trying to reconstruct the original signals 'x' from 'y' using the Reconstruction equation ( f(x) ~= \sum_{n} c_n(t) (2n+1)^(1/2) Pn(2x/t-1) ) in Appendix D.3, the result seems different, but connected in some way (e.g., suppose t=1, calculating \sum{n} y[0,0,n] (2n+1)^(1/2) P_n(2-1) gives twice the target number 'x[0]' I want)...Probably there is a misunderstanding of mine?