Skip to content

[kalman] update RV notation#900

Open
longye-tian wants to merge 5 commits into
mainfrom
kal_rv
Open

[kalman] update RV notation#900
longye-tian wants to merge 5 commits into
mainfrom
kal_rv

Conversation

@longye-tian

Copy link
Copy Markdown
Contributor

Hi @mmcky

This PR updates the RV notation following #898 .

In particular, this extends the new uppercase/lowercase convention to the Implementation section and Exercises:

  • use $X_t$ and $Y_t$ for random state and signal variables;
  • keep lowercase notation for realizations, density arguments, filter estimates, and code variables;
  • clarify that the Exercise 3 plotted errors use simulated realizations.

Best,
Longye

longye-tian and others added 3 commits June 8, 2026 17:16
Update the two "Constant value of state x_t" comments to X_t so the
code comments match the uppercase random-variable convention used in
the surrounding prose. The realized initial value x_0 comment stays
lowercase by design.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mmcky

mmcky commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Reviewed — the uppercase/lowercase RV convention is applied correctly and consistently, and the new x[:, t] is the realized value of $X_t$ note nicely bridges the math and the Ex3 code.

I pushed a small follow-up commit (dc7ca23) updating the two # Constant value of state x_t code comments to X_t to match. LGTM once CI is green.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

📖 Netlify Preview Ready!

Preview URL: https://pr-900--sunny-cactus-210e3e.netlify.app

Commit: 23a0563

📚 Changed Lectures


Build Info

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Kalman filter lecture (lectures/kalman.md) to consistently apply the “uppercase = random variable / lowercase = realization” convention across the Implementation section and Exercises, aligning with the notation introduced earlier in the lecture and requested in #898.

Changes:

  • Updated the LinearStateSpace model equations in the Implementation section to use $X_t$ / $Y_t$ for random state and signal variables.
  • Revised Exercises 1–4 text and code comments to use $X_t$ for random states while keeping lowercase for realizations/estimates/code variables.
  • Clarified in Exercise 3 that the plotted squared errors are based on simulated realizations (and added an explicit note tying x[:, t] to the realized $X_t$).

Comment thread lectures/kalman.md Outdated
Comment thread lectures/kalman.md
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@longye-tian longye-tian added enhancement New feature or request ready labels Jun 12, 2026
@longye-tian

Copy link
Copy Markdown
Contributor Author

Many thanks @mmcky for the update and I've addressed the comments from Copilot as well.

Best,
Longye

@mmcky

mmcky commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Hi @longye-tian — I've resolved the merge conflict on this branch (merged latest main in via 23a0563), but I'd like you to double-check the resolution since it involved harmonising with another notation change.

The conflict arose because #908 ("[kalman] Fix errors, modernize code, and unify notation") landed on main after this branch was created, and it did a global notation overhaul of the same file: \hat x → \mu (e.g. \hat x_t → \mu_t, code variable x_hat → μ) and the state shock w_t → W_t. That collided with your RV-uppercase changes in three places.

I resolved each by keeping your changes and adopting main's now-established notation where they overlapped, so the file stays internally consistent. Specifically:

Location How I resolved it
Exercise 1 task Kept your grammar fix ("exercise is to") and "for $X_t$"; used main's N(\mu_t, \Sigma_t) instead of N(\hat x_t, \Sigma_t)
Exercise 3 intro Kept your uppercase $X_t$ / $X_{t-1}$ for the random variables; used main's $\{W_t\}$ for the shock sequence and $\mu_t$ for the filter prediction
Exercise 3 task Kept your "simulated realizations", uppercase $X_t$ / $X_{t-1}$, and your new "x[:, t] is the realized value of $X_t$" note; used main's $\mu_t$ in $| X_t - \mu_t |^2$

The one point worth your eye: #908 set the state shock to uppercase $W_t$ (in kl_xdynam and the Convergence section), so in Exercise 3 I used $\{W_t\}$ to match. Your state-space block still keeps lowercase $w_t$ / $v_t$ for the standard-normal shocks of the general LinearStateSpace form, which I left as you wrote it. If you'd prefer a single convention for shocks throughout, let me know and I can align it.

Could you confirm the resolution reads the way you intended? Thanks!

@mmcky mmcky removed the ready label Jun 18, 2026
@jstac

jstac commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

@longye-tian My bad, I missed this PR and then made some related edits. Sorry about that.

Could you please provide a brief review of the current status of the lecture and this PR?

@longye-tian

Copy link
Copy Markdown
Contributor Author

Hi @longye-tian — I've resolved the merge conflict on this branch (merged latest main in via 23a0563), but I'd like you to double-check the resolution since it involved harmonising with another notation change.

The conflict arose because #908 ("[kalman] Fix errors, modernize code, and unify notation") landed on main after this branch was created, and it did a global notation overhaul of the same file: \hat x → \mu (e.g. \hat x_t → \mu_t, code variable x_hat → μ) and the state shock w_t → W_t. That collided with your RV-uppercase changes in three places.

I resolved each by keeping your changes and adopting main's now-established notation where they overlapped, so the file stays internally consistent. Specifically:

Location How I resolved it
Exercise 1 task Kept your grammar fix ("exercise is to") and "**for X t **"; used main's N(\mu_t, \Sigma_t) instead of N(\hat x_t, \Sigma_t)
Exercise 3 intro Kept your uppercase $X_t$ / $X_{t-1}$ for the random variables; used main's $\{W_t\}$ for the shock sequence and $\mu_t$ for the filter prediction
Exercise 3 task Kept your "simulated realizations", uppercase $X_t$ / $X_{t-1}$, and your new "x[:, t] is the realized value of
𝑋
𝑡
" note; used main's $\mu_t$ in $| X_t - \mu_t |^2$
The one point worth your eye: #908 set the state shock to uppercase $W_t$ (in kl_xdynam and the Convergence section), so in Exercise 3 I used $\{W_t\}$ to match. Your state-space block still keeps lowercase $w_t$ / $v_t$ for the standard-normal shocks of the general LinearStateSpace form, which I left as you wrote it. If you'd prefer a single convention for shocks throughout, let me know and I can align it.

Could you confirm the resolution reads the way you intended? Thanks!

Many thanks @mmcky They all looks good to me!

@longye-tian

Copy link
Copy Markdown
Contributor Author

@longye-tian My bad, I missed this PR and then made some related edits. Sorry about that.

Could you please provide a brief review of the current status of the lecture and this PR?

No worries @jstac.

I think @mmcky has resolved all the merge conflicts with your PR #908 .

This PR and yours now share the same notations and this PR complements yours by focuses on the Implementation section and Exercises section.

Best,
Longye

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

4 participants