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

Improve numerical stability of the Kalman filter for ARIMA #4259

Merged
merged 11 commits into from
Oct 26, 2021

Conversation

Nyrio
Copy link
Contributor

@Nyrio Nyrio commented Oct 1, 2021

This PR improves the numerical stability of the Kalman filter by enforcing that the state covariance matrix is always symmetric and only has positive elements along its diagonal. Note that statsmodels does the former and not the latter, but I have found both to improve the stability in cases I was testing.

If stability problems arise in the future, a strategy to consider would be to use Joseph form for the covariance update. This avoids the subtraction which can result in a loss of positive-definiteness, but at the expense of performance because this formula is more complex.

@Nyrio Nyrio added 2 - In Progress Currenty a work in progress CUDA / C++ CUDA issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 1, 2021
@Nyrio Nyrio requested a review from a team as a code owner October 1, 2021 17:05
@Nyrio Nyrio added this to PR-WIP in v21.12 Release via automation Oct 1, 2021
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Oct 4, 2021
@Nyrio Nyrio added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currenty a work in progress labels Oct 4, 2021
@Nyrio Nyrio requested a review from a team as a code owner October 12, 2021 09:32
@tfeher tfeher self-assigned this Oct 12, 2021
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Hi @Nyrio , thanks for this PR! It looks good, I just some smaller suggestions. In case we would need to return to this in the future: are there any references / links / keywords about the Kalman filter stability problem that might be worth adding to the PR description?

cpp/src/arima/batched_kalman.cu Outdated Show resolved Hide resolved
cpp/src_prims/linalg/block.cuh Outdated Show resolved Hide resolved
v21.12 Release automation moved this from PR-WIP to PR-Needs review Oct 13, 2021
@Nyrio
Copy link
Contributor Author

Nyrio commented Oct 13, 2021

@tfeher Thanks for the review. I have updated the PR.

In case we would need to return to this in the future: are there any references / links / keywords about the Kalman filter stability problem that might be worth adding to the PR description?

I added a mention of the Joseph form. The main numerical instability source in the Kalman filter is the covariance update, and this is especially true with the simplified formula where the subtraction can result in a loss of positive-definiteness. Hence the Joseph formula is a possible (expensive) fix.

Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks @Nyrio for the update! The PR looks good to me.

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.12@ee2e863). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.12    #4259   +/-   ##
===============================================
  Coverage                ?   85.98%           
===============================================
  Files                   ?      231           
  Lines                   ?    18577           
  Branches                ?        0           
===============================================
  Hits                    ?    15974           
  Misses                  ?     2603           
  Partials                ?        0           
Flag Coverage Δ
dask 46.67% <0.00%> (?)
non-dask 78.62% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee2e863...2736f8b. Read the comment docs.

v21.12 Release automation moved this from PR-Needs review to PR-Reviewer approved Oct 26, 2021
@dantegd
Copy link
Member

dantegd commented Oct 26, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 4775124 into rapidsai:branch-21.12 Oct 26, 2021
v21.12 Release automation moved this from PR-Reviewer approved to Done Oct 26, 2021
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
…4259)

This PR improves the numerical stability of the Kalman filter by enforcing that the state covariance matrix is always symmetric and only has positive elements along its diagonal. Note that statsmodels does the former and not the latter, but I have found both to improve the stability in cases I was testing.

If stability problems arise in the future, a strategy to consider would be to use [Joseph form](https://en.wikipedia.org/wiki/Kalman_filter#Deriving_the_posteriori_estimate_covariance_matrix) for the covariance update. This avoids the subtraction which can result in a loss of positive-definiteness, but at the expense of performance because this formula is more complex.

Authors:
  - Louis Sugy (https://github.com/Nyrio)

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#4259
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team CUDA / C++ CUDA issue CUDA/C++ Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants