A different CARMA kernel implementation #90

ywx649999311 · 2022-04-29T00:20:27Z

I added my implementation of the CARMA kernel (I named it lower case carma), which basically follows the suggestion in your celerite paper and breaks a CARMA model into a combination of real exponential kernels and celerite kernels. I try to follow the notation in Kelly+14 CARMA paper but absorbed its sigma term into the beta parameters.

I have also added a bunch of tests to check the CARMA and carma kernels using the equivalent exponential and celerite kernels in test_quasisep.py. In test_kalman.py, I added a few more CARMA specifications that will break your tests there.

To note that the current implementation will only work well for CARMA(2,1) or lower. To fit any higher order CARMA models, we need a separate constructor that will sample in the root space rather than the CARMA space (see discussion in section 3.3 in the Kelly+14 paper). I am working on migrating my EzTao code here and will keep developing on this branch.

src/tinygp/kernels/quasisep.py

tests/test_solvers/test_kalman.py

dfm · 2022-05-02T14:50:33Z

Thanks for this! I added a few inline comments to get us started.

It would also be great to add some tests that demonstrate the issues from #89, that get fixed by this PR.

To note that the current implementation will only work well for CARMA(2,1) or lower. To fit any higher order CARMA models, we need a separate constructor that will sample in the root space rather than the CARMA space (see discussion in section 3.3 in the Kelly+14 paper). I am working on migrating my EzTao code here and will keep developing on this branch.

Is there any reason to not just always operate in that parameterization? A limit of 2,1 seems pretty restrictive!

ywx649999311 · 2022-05-03T00:33:34Z

It would also be great to add some tests that demonstrate the issues from #89, that get fixed by this PR.

Good point. I will work on that.

Is there any reason to not just always operate in that parameterization? A limit of 2,1 seems pretty restrictive!

That is because not any combination of CARMA parameters returns a valid model, that is stationary and in the so-called 'minimum phase'. An -inf likelihood will (likely) be returned if the input model is not valid. This also relates to the requirement for the log input of alphas and betas. After all, we need the roots of the autoregressive and moving-average polynomials (e.g., Equation 30 of the Kelly+14 paper) to have negative real parts in order to generate a 'valid' model.

dfm · 2022-05-03T13:11:20Z

That is because not any combination of CARMA parameters returns a valid model, that is stationary and in the so-called 'minimum phase'. An -inf likelihood will (likely) be returned if the input model is not valid. This also relates to the requirement for the log input of alphas and betas. After all, we need the roots of the autoregressive and moving-average polynomials (e.g., Equation 30 of the Kelly+14 paper) to have negative real parts in order to generate a 'valid' model.

The approach I would take to this is to clearly document (probably by adding a tutorial, as well as clear docstrings) different approaches for parameterizing valid models.

dfm · 2022-05-18T22:25:13Z

This is on my radar, but I've been totally swamped recently. Thanks for your patience!

ywx649999311 · 2022-05-19T15:12:52Z

This is on my radar, but I've been totally swamped recently. Thanks for your patience!

No problem, me too. I am not done making changes yet, will bug you once I am ready. Thanks!

ywx649999311 · 2022-05-21T01:16:26Z

Hi, please give a look when you get time. I removed the old implementation and tests that I created for it. I think there might be a need to make a tutorial to explain the two different way to parameterize the model, and I can work on it once this is merged.

dfm · 2022-10-30T23:46:25Z

@ywx649999311 — I'm so sorry about taking so long to get back to this!! I've just fixed the merge conflicts and fixed some typos so you should pull before making any more changes!

dfm · 2022-10-30T23:47:00Z

~~You might also consider rebasing onto main, but that's definitely not required.~~

Scratch that - I did the rebase. Sorry if it's rude to force push directly to your branch! I'll try not to do it again...

More comments incoming!

dfm

I've left a few more small comments, but I think this is looking great!! Thanks again for submitting this, and thanks for your patience.

src/tinygp/kernels/quasisep.py

dfm · 2022-10-31T00:10:59Z

src/tinygp/kernels/quasisep.py

+        The roots can be re-parameterized as the coefficients of a product
+        of quadratic equations each with the second-order term set to 1. The
+        input for this constructor are said coefficients. See Equation 30 in
+        the paper linked above for a reference.


It's probably better to directly link to the paper here, instead of referencing to above.

OK, fixed it.

src/tinygp/kernels/quasisep.py

dfm · 2022-10-31T00:16:11Z

@ywx649999311 — Besides my small comments above, I'm pretty much ready to merge this. I'd love it if you could add a little more detail to all the docstrings just because I'm not super familiar with all the details of this model and I want to make sure I can maintain it :D

dhuppenkothen · 2023-10-17T07:18:42Z

Just wanted to say I've been using this branch to fit a CARMA(2,1) model, and it seems more stable than the current CARMA implementation in TinyGP (where I get a lot of NaNs and my attempts at sampling a posterior have had mixed results).

I would love to see this extended to higher-order CARMA processes and would be happy to help in whatever capacity I'm capable of doing so given my limited knowledge of JAX.

This reverts commit 038ace5

for more information, see https://pre-commit.ci

updates: - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0) - [github.com/psf/black: 23.9.1 → 23.10.1](psf/black@23.9.1...23.10.1) - [github.com/astral-sh/ruff-pre-commit: v0.0.292 → v0.1.4](astral-sh/ruff-pre-commit@v0.0.292...v0.1.4) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

for more information, see https://pre-commit.ci

ywx649999311 · 2023-11-08T17:16:12Z

Hi @dfm, I added more comments/docstring. Would you please see if you have further comments? Thanks!

Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.8.10 to 1.8.11. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.8.10...v1.8.11) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

updates: - [github.com/psf/black: 23.10.1 → 23.11.0](psf/black@23.10.1...23.11.0) - [github.com/astral-sh/ruff-pre-commit: v0.1.4 → v0.1.6](astral-sh/ruff-pre-commit@v0.1.4...v0.1.6) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 3 to 4. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 3 to 4. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4 to 5. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

updates: - [github.com/psf/black: 23.11.0 → 23.12.1](psf/black@23.11.0...23.12.1) - [github.com/astral-sh/ruff-pre-commit: v0.1.6 → v0.1.9](astral-sh/ruff-pre-commit@v0.1.6...v0.1.9) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

dfm · 2024-01-04T01:18:55Z

@ywx649999311 — I'm so sorry for the delay with this!! I think it looks great. I just went through and made some small changes to the structure. In particular, I managed to remove the eta parameter by using a where instead that should be more stable. I'll leave some comments inline highlighting the main changes that you should check. As soon as you're happy, I'll merge. Thanks again so much for all your work on this!!!

src/tinygp/kernels/quasisep.py

ywx649999311 · 2024-01-04T18:10:47Z

Hi @dfm,
I think it all looks awesome! I added comments regarding the use of [::2] in jnp.ravel(jnp.array([a, b]))[::2]. Thanks for helping with getting rid of eta. I agree the updated implementation is clearer and more stable. Please merge it when you can. I am very happy that this kernel can be used by more people!

dfm · 2024-01-04T19:03:08Z

Merged!! 🚀

Thanks again @ywx649999311, this is such a great addition!!

dfm · 2024-01-04T19:05:53Z

@ywx649999311 — One other thing: could you open another PR with your name, affiliation and orcid in https://github.com/dfm/tinygp/blob/main/.zenodo.json? I meant to get that as part of this PR, but I forgot. Sorry!

ywx649999311 · 2024-01-04T19:55:48Z

@dfm I created a new PR, thanks!

dfm reviewed May 2, 2022

View reviewed changes

src/tinygp/kernels/quasisep.py Outdated Show resolved Hide resolved

src/tinygp/kernels/quasisep.py Outdated Show resolved Hide resolved

dfm reviewed May 2, 2022

View reviewed changes

tests/test_solvers/test_kalman.py Outdated Show resolved Hide resolved

ywx649999311 requested a review from dfm May 21, 2022 01:16

ywx649999311 mentioned this pull request Aug 11, 2022

JAX implementation -> prototype ywx649999311/EzTao#61

Closed

dfm force-pushed the carma branch from 1ae552b to 0df6b4e Compare October 30, 2022 23:54

dfm requested changes Oct 31, 2022

View reviewed changes

dfm self-assigned this Oct 31, 2022

dhuppenkothen mentioned this pull request Oct 4, 2023

Attempt to make CARMA kernel jit-compliant #188

Merged

ywx649999311 and others added 12 commits November 2, 2023 16:16

solve conflict in test files

b7a9450

bring back accidentally removed code

cb0e4fe

Revert 'bring back accidentally removed code'

70418c0

This reverts commit 038ace5

remove test code

f0e1271

add comments

83d768d

Remove new CARMA tests from test_kalman

083a8b3

carma kernel now takes normal params (no log needed)

ba4ae5a

Add tests for issue dfm#89

cb0014e

cleanup; match caram notaion to kelly+14

c68474e

[pre-commit.ci] auto fixes from pre-commit.com hooks

7b4b3cd

for more information, see https://pre-commit.ci

Add second CARMA constructor and tests

44b9c44

solve conflict in quasisep.py

13efa2d

ywx649999311 and others added 2 commits November 2, 2023 16:24

fixed white space lint

3911df4

update typing notation

00f556b

ywx649999311 force-pushed the carma branch from 60dde94 to 00f556b Compare November 6, 2023 19:49

pre-commit-ci bot and others added 8 commits November 6, 2023 19:49

[pre-commit.ci] auto fixes from pre-commit.com hooks

b878e45

for more information, see https://pre-commit.ci

fix lint and doc error

959522e

update docstring

536eed0

[pre-commit.ci] auto fixes from pre-commit.com hooks

e0ceece

for more information, see https://pre-commit.ci

fix doc now showing math errors

89cc321

more doc str

f0f5d5b

[pre-commit.ci] auto fixes from pre-commit.com hooks

599371c

for more information, see https://pre-commit.ci

ywx649999311 requested a review from dfm November 8, 2023 17:16

dependabot bot and others added 8 commits December 3, 2023 13:10

Merge branch 'carma' of https://github.com/ywx649999311/tinygp

f049cbc

Simplifying masking logic, fixing some typos

87e2a93

dfm reviewed Jan 4, 2024

View reviewed changes

src/tinygp/kernels/quasisep.py Show resolved Hide resolved

src/tinygp/kernels/quasisep.py Show resolved Hide resolved

src/tinygp/kernels/quasisep.py Show resolved Hide resolved

src/tinygp/kernels/quasisep.py Show resolved Hide resolved

src/tinygp/kernels/quasisep.py Show resolved Hide resolved

add comments for jnp.ravel(om_complex)[::2]

dcad867

dfm approved these changes Jan 4, 2024

View reviewed changes

dfm merged commit 3f49239 into dfm:main Jan 4, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A different CARMA kernel implementation #90

A different CARMA kernel implementation #90

ywx649999311 commented Apr 29, 2022

dfm commented May 2, 2022

ywx649999311 commented May 3, 2022

dfm commented May 3, 2022

dfm commented May 18, 2022

ywx649999311 commented May 19, 2022

ywx649999311 commented May 21, 2022

dfm commented Oct 30, 2022

dfm commented Oct 30, 2022 •

edited

Loading

dfm left a comment

dfm Oct 31, 2022

ywx649999311 Jan 20, 2023

dfm commented Oct 31, 2022

dhuppenkothen commented Oct 17, 2023

ywx649999311 commented Nov 8, 2023 •

edited

Loading

dfm commented Jan 4, 2024

ywx649999311 commented Jan 4, 2024

dfm commented Jan 4, 2024

dfm commented Jan 4, 2024

ywx649999311 commented Jan 4, 2024

A different CARMA kernel implementation #90

A different CARMA kernel implementation #90

Conversation

ywx649999311 commented Apr 29, 2022

dfm commented May 2, 2022

ywx649999311 commented May 3, 2022

dfm commented May 3, 2022

dfm commented May 18, 2022

ywx649999311 commented May 19, 2022

ywx649999311 commented May 21, 2022

dfm commented Oct 30, 2022

dfm commented Oct 30, 2022 • edited Loading

dfm left a comment

Choose a reason for hiding this comment

dfm Oct 31, 2022

Choose a reason for hiding this comment

ywx649999311 Jan 20, 2023

Choose a reason for hiding this comment

dfm commented Oct 31, 2022

dhuppenkothen commented Oct 17, 2023

ywx649999311 commented Nov 8, 2023 • edited Loading

dfm commented Jan 4, 2024

ywx649999311 commented Jan 4, 2024

dfm commented Jan 4, 2024

dfm commented Jan 4, 2024

ywx649999311 commented Jan 4, 2024

dfm commented Oct 30, 2022 •

edited

Loading

ywx649999311 commented Nov 8, 2023 •

edited

Loading