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

Experiment reproduction issue with updated modules #29

Closed
jhuang265 opened this issue May 9, 2022 · 5 comments
Closed

Experiment reproduction issue with updated modules #29

jhuang265 opened this issue May 9, 2022 · 5 comments

Comments

@jhuang265
Copy link

Hi, I was trying to reproduce some of your results using the SaShiMi model by running the command

python -m train experiment=sashimi-sc09 wandb=null

but I get the error

TypeError: __init__() got an unexpected keyword argument 'pool'

due to the DownPool class no longer needing pool parameter for initialization.

Can I ask if there are any plans to fix these issues so that they work with the current implementations of the different modules?

@albertfgu
Copy link
Contributor

Nice catch! Can you pull and try again?

@jhuang265
Copy link
Author

jhuang265 commented May 9, 2022

I happen to be getting a different issue now

TypeError: __init__() got an unexpected keyword argument 'hurwitz'

I can see that this is one of the parameters in the S4 block that was removed in one of the recent updates to the project, so I would assume that this might pop up in some other experiments as well if they weren't updated.

@albertfgu
Copy link
Contributor

You're right, I just pushed another fix for this.

Actually, I can't test right now on my local environment. The model API did change very slightly in the latest release and I wasn't able to test the Sashimi experiments, so there may be some small bugs in the configs. These should only be very small tweaks though in the model and configs. If you could figure out the necessary changes to get the model to run and report back, that would be extremely helpful :)

@jhuang265
Copy link
Author

I think your most recent commit fixed the issues with the experiments by just commenting out the hurwitz parameter in the config files.

The stand-alone SaShiMi module still has an issue where in setup_rnn, the mode is being passed to the submodules in the SaShiMi model on lines 443-444

for module in self.modules():
    if hasattr(module, 'setup_step'): module.setup_step(mode)

but this is passed to the standalone S4 module which doesn't take in the mode as a parameter. I just changed the signature of each setup_step function (in S4 and HippoSSKernel) to pass in the mode downwards into the SSKernelNPLR module for the time being. This is a pretty hacky fix though so I'm sure you might want to look into if there's a better way to handle this later. This leads to another error

torch._C._LinAlgError: linalg.solve: (Batch element 0): The diagonal element 1 is zero, the solve could not be completed because the input matrix is singular.

which I didn't run into beforehand when I tried python sashimi.py prior to the modifications to the standalone S4 kernel implementation.

@albertfgu
Copy link
Contributor

V3 should have fixed any potential issues here. Feel free to re-open an issue if you find any further problems.

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