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

Ridge multiple lags #11

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Ridge multiple lags #11

wants to merge 17 commits into from

Conversation

jannesvaningen
Copy link

@jannesvaningen jannesvaningen commented Jul 5, 2023

Some last minute fixes for reading presentation in:

  • readme for installation
  • s2spy added to dependencies
  • path of model outputs
  • timeseries plots for lstm
  • Added a lags for loop in the ridge ipynb to accomodate fitting a ridge model for multiple lags add once.
    I also added a line plot to show the y_train and y_test outcomes of the model.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -34,23 +34,13 @@
},
Copy link
Member

@geek-yang geek-yang Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line #8.        f"The MSE of climatology is {mean_squared_error(ground_truth, np.repeat(target_clim, ground_truth.anchor_year.size)):.3f}"

Thanks for correcting it. I forgot to update this with climatology. Btw, for a fair evaluation, I think when calculating the climatology, we shall only include the training data, instead of the whole dataset (if you are looking for something else, just ignore my comment).


Reply via ReviewNB

Copy link
Member

@geek-yang geek-yang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for adding the multi lag regression to our recipes for a fair comparison. I planned to do it but didn't manage to find time. 😉

Just a small comment, it seems the deep learning ones are not very robust. Maybe we can add random seeds, to make the results reproducible, for instance:

torch.manual_seed(1)
# for python related randomness
random.seed(1)
# for numpy related randomness
np.random.seed(1)

(more information https://pytorch.org/docs/stable/notes/randomness.html)

Nice work @jannesvaningen !

Copy link
Member

@semvijverberg semvijverberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I check all notebooks, fixed some merge issue related to later updates (fixed seed + downloading data).

+Now computing mean_squared_error for every notebook in the same manner.

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

Successfully merging this pull request may close these issues.

3 participants