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

reconstruction error #16

Open
q-learning-trader opened this issue May 21, 2021 · 8 comments
Open

reconstruction error #16

q-learning-trader opened this issue May 21, 2021 · 8 comments
Labels
good first issue Good for newcomers invalid This doesn't seem right

Comments

@q-learning-trader
Copy link

Hi, nice project.
Thanks for sharing.
I am looking around and testing.
It seems get_loaders function does not fit run_reconstruction.
In particular the following line:

X_train, X_test, y_train, y_test = self.preprocess_data()

it is not correct for reconstruction. For reconstruction we do not need y values.

@JulesBelveze JulesBelveze added good first issue Good for newcomers invalid This doesn't seem right labels May 21, 2021
@JulesBelveze
Copy link
Owner

Hey @q-learning-trader thanks for pointing that out.

You're actually right, this doesn't seem right to me either.

Would you like opening a PR to fix this? 😃

@q-learning-trader
Copy link
Author

I might try if I have time @JulesBelveze .
I had a look to the model and the model seems it fits prediction but not reconstruction.
In reconstruction you should have same shape for features and labels while in prediction labels has shape one because you are trying to predict a time series.
Do you agree?

@JulesBelveze
Copy link
Owner

You are totally right @q-learning-trader
I think the model is fine, the problem is the way the data is processed and the Dataset is built.

@q-learning-trader
Copy link
Author

I have tried to make some simple changes in the Dataset and I am now getting the following error:

RuntimeError: size mismatch, m1: [16 x 77], m2: [65 x 1] at /opt/conda/conda-bld/pytorch_1591914880026/work/aten/src/TH/generic/THTensorMath.cpp:41

For instance in the the decoder, the following forward return I do not think it fits the reconstruction:

return self.fc_out(torch.cat((h_t[0], context.to(device)), dim=1)) # predicting value at t=self.seq_length+1

dim=1 I do not think is good in this case.
What do you think?

@JulesBelveze
Copy link
Owner

@q-learning-trader I haven't looked at that code for a while so I would need to investigate further.
But you definitely pointed something that seems weird.

I would be grateful if you can look into that otherwise I will try to spend some on this :)

@MatthewWaller
Copy link

Ha! I ran into this issue too. Couldn’t make reconstruction work in the sample config setup. Also tried removing the y values and ran into the size mismatch. Hmmm

@JulesBelveze
Copy link
Owner

Hey @MatthewWaller,
Thanks for using this repo! I'm planning on addressing the opened issues at the beginning of next month. Will keep you posted 😃

@afcarzero1
Copy link

Is this still open? It looks to me that the problem is into the definition of y_hist.

I think y is defined correctly because it should be the target but y_hist looks weird to me.

                y_hist.append(torch.FloatTensor(X[i - 1: i + self.seq_length - 1, :]).unsqueeze(0))
                target.append(
                    torch.FloatTensor(X[i + self.seq_length:i + self.seq_length + self.prediction_window, :]))

I think it should be

 y_hist.append(torch.FloatTensor(X[i : i + self.seq_length, :]).unsqueeze(0))

(Maybe I am missing something)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants