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

Sparse Reconciliation #210

Merged
merged 9 commits into from
Jul 13, 2023
Merged

Sparse Reconciliation #210

merged 9 commits into from
Jul 13, 2023

Conversation

mcsqr
Copy link
Contributor

@mcsqr mcsqr commented Jun 15, 2023

Using sparse matrices to reduce the computational time and memory appetite of a subset of the reconciliation methods.

The PR contains MinTraceSparse which supports the diagonal methods (ols, wls_struct, and wls_var), the speed-up is very considerable for datasets of ~2k-20k time series (the most I checked so far).

It also contains BottomUpSparse, which doesn't really show speed-up until 20k, but should hopefully help with even bigger datasets.

Note: This version still doesn't guarantee that no O(N^2) dense matrix is instantiated, it could probably be checked/improved by testing on even bigger datasets.

Also likely some docs and tests are missing, tbd.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2023

CLA assistant check
All committers have signed the CLA.

@kdgutier kdgutier self-requested a review June 15, 2023 14:26
@kdgutier
Copy link
Collaborator

Thanks, @mcsqr
This is awesome; we will take a look.

From the execution of the CI tests, we will need to fix the scikit-learn version to have the updated sparse OneHotEncoder. This would need to be done here:
https://github.com/Nixtla/hierarchicalforecast/blob/main/environment.yml
https://github.com/Nixtla/hierarchicalforecast/blob/main/settings.ini
I don't know if we will need to increase min_python > 3.7

@AzulGarza AzulGarza self-requested a review June 15, 2023 15:29
@mcsqr
Copy link
Contributor Author

mcsqr commented Jun 16, 2023

CI tests are fixed. Note that sparse argument in OneHotEncoder is deprecated and scheduled for removal, so at some point it'll also break, but for now it looks ok.

Copy link
Collaborator

@kdgutier kdgutier left a comment

Choose a reason for hiding this comment

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

Awesome @mcsqr,

My most important comment is on the declaration of S_sparse in core.py.
Thanks a lot for your contribution.

hierarchicalforecast/utils.py Show resolved Hide resolved
hierarchicalforecast/core.py Outdated Show resolved Hide resolved
@mcsqr mcsqr changed the title DRAFT: Sparse Reconciliation Sparse Reconciliation Jul 13, 2023
@mcsqr mcsqr marked this pull request as ready for review July 13, 2023 05:39
@AzulGarza AzulGarza merged commit fe7ce51 into Nixtla:main Jul 13, 2023
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.

4 participants