Skip to content

A possible subtle mistake related to code implementation of nplr #159

@MichaelY310

Description

@MichaelY310

In s4/src/models/hippo/hippo.py line 240

W_im, V = torch.linalg.eigh(AP*-1j) # (..., N) (..., N, N)

to calculate the complex part of the eigenvalue and eigenvectors, instead of AP, the skew symmetric part, which is AP - torch.diag(torch.diag(AP)), should be eigendecomposed. So I think this line should be replaced by

W_im, V = torch.linalg.eigh((AP-torch.diag(torch.diag(AP)))*-1j)

However, the mistake is very subtle here since the diagonal value -0.5 is very small comparing to other entries in AP. The revision only decreases the reconstruction error of A by about 0.00001 if I set cdtype to torch.complex128. And there is no difference at all if I use the default setting torch.complex64. But if the mistake indeed present, it may cause greater errors when methods other than LegS with matrices having small matrix norms are used.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions