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.

Sign up for GitHub

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

Jump to bottom

LGSSM notebook Latex and Code Mismatch and other improvements #308

Open
canyon289 opened this issue Mar 27, 2023 · 3 comments · May be fixed by #312
Open

LGSSM notebook Latex and Code Mismatch and other improvements #308

canyon289 opened this issue Mar 27, 2023 · 3 comments · May be fixed by #312

Comments

@canyon289
Copy link
Contributor

canyon289 commented Mar 27, 2023 •
edited
Loading

I believe the order of the indices in the code versus the latex code are different for the lgssm docs, where in the code the positions are the first two indices as

Latex

$$\underbrace{\begin{pmatrix} u_t\\ \dot{u}_t \\ v_t \\ \dot{v}_t \end{pmatrix}}_{z_t}$$

Code

$$\underbrace{\begin{pmatrix} u_t\\ v_t \\ \dot{u}_t \\\dot{v}_t \end{pmatrix}}_{z_t}$$

In addition to this the docs could generally be improved by labeling the axes figures so its more apparent what is what, and adding comments that certain covariances were scaled to be quite small so the true movement would be "straight' whereas others were multiplied by a float so jax won't complain.

Happy to take this one if I

  1. Am indeed correct that the notebook disagrees with itself
  2. That additional fixes to the documentation are welcome

Screenshots

image

image
@canyon289 canyon289 changed the title LGMSSM Latex and Code Mismath and other improvements LGMSSM notebook Latex and Code Mismatch and other improvements Mar 27, 2023
@canyon289 canyon289 changed the title LGMSSM notebook Latex and Code Mismatch and other improvements LGSSM notebook Latex and Code Mismatch and other improvements Mar 27, 2023
@gileshd
Copy link
Collaborator

gileshd commented Apr 7, 2023 •
edited
Loading

Hi @canyon289! Nice catch!

Yes I think there is a problem in the equation in the docs which should read:

$$\underbrace{\begin{pmatrix} u_t\\ v_t \\ \dot{u}_t \\\dot{v}_t \end{pmatrix}}_{z_t}$$

this would involve also updating the following equation to read:

$$\begin{align*} \underbrace{\begin{pmatrix} y_{1,t} \\ y_{2,t} \end{pmatrix}}_{y_t} &= \underbrace{ \begin{pmatrix} 1 & 0 & 0 & 0 \\\ 0 & 1 & 0 & 0 \end{pmatrix} }_{H} \underbrace{\begin{pmatrix} u_t\\ v_t \\ \dot{u}_t \\ \dot{v}_t \end{pmatrix}}_{z_t} + r_t \end{align*}$$

If you are happy to give this a shot go ahead and open a PR 👍 and feel free to also add axis labels.

re multiplying by a float so that jax doesn't complain are you referring to:

emission_covariance = jnp.eye(emission_dim) * 1.0

If so I think that the * 1.0 is probably superfluous because, as far as I know, jnp.eye() returns floats anyway.

@canyon289
Copy link
Contributor Author

Thanks for confirming. Ill be happy to open a PR. Please give me till Sunday evening

@canyon289
Copy link
Contributor Author

My apologies give me a couple more days. The week got away from me

@canyon289 canyon289 linked a pull request Apr 17, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update notebook canyon289/dynamax
2 participants
@canyon289 @gileshd