ORNL / AADL

Anderson Acceleration for Deep Learning
12 stars 6 forks source link

Typo in anderson_acceleration.py ? #2

Closed henrymai closed 3 years ago

henrymai commented 3 years ago

https://github.com/ORNL/AADL/blob/59c19f7c221ebda434359ddd7582399b925a3722/AADL/anderson_acceleration.py#L19

File "[...] anderson_acceleration.py", line 19, in anderson_qr_factorization
  gamma = torch.linalg.lstsq(DR, R[:, -1]).solution
NameError: name 'R' is not defined

For that line, should R be DR instead?

henrymai commented 3 years ago

My pull request here (for history offload to cpu memory) includes a fix for this typo: https://github.com/ORNL/AADL/pull/3

allaffa commented 3 years ago

Hi! First of all, thank you very much for contributing :-)

I attach here a screenshot from a presentation that describes how Anderson Acceleration is defined

Screen Shot 2021-10-29 at 10 15 09 AM

What is defined as W in the slide, the code defines as DX What is defined as R in the slide, the code defines as DR The coefficient matrix DR is constructed so that every column is the difference between residuals computed at consecutive iterations.

The right-hand-side is the residual at the very last iteration.

So I think that line 19 of anderson_acceleration.py is define correctly

henrymai commented 3 years ago

Hi,

If line 19 is correct, where is R defined? Did you see the error I included in the post?

Never mind, @allaffa answered in my pull request comment. I submitted a new PR to change the right hand side to DX now.