[additive_functionals] Style guide review #268
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=80&v=4){kind=small}
- admon: 4 fixes - code: 3 fixes - jax: 2 fixes - link: 5 fixes - math: 3 fixes - writing: 34 fixes Rules addressed: - qe-math-001: Prefer UTF-8 unicode for simple parameter mentions, be consistent - qe-math-002: Use \top for transpose notation - qe-math-004: Do not use bold face for matrices or vectors - qe-code-002: Use Unicode symbols for Greek letters in code - qe-code-002: Use Unicode symbols for Greek letters in code - qe-code-002: Use Unicode symbols for Greek letters in code - qe-jax-002: Use NamedTuple for model parameters - qe-jax-002: Use NamedTuple for model parameters - qe-link-002: Use doc links for cross-series references - qe-link-002: Use doc links for cross-series references - ... and 41 more
✅ Deploy Preview for lustrous-melomakarona-3ee73e ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📋 Style Guide Review: additive_functionals
This PR addresses style guide compliance issues found in the
additive_functionalslecture.📊 Summary
🎯 Issues by Priority
📝 Detailed Changes
Admon (4 issues)
qe-admon-001 - Use gated syntax for executable code in exercises
N/A...N/A...qe-admon-003 - Use tick count management for nested directives
{raw}directive but does not have any nested directives within it that would require tick count management. No violation found.N/A...N/A...qe-admon-004 - Use prf prefix for proof directives
N/A...N/A...qe-admon-005 - Link solutions to exercises
N/A...N/A...Code (3 issues)
qe-code-002 - Use Unicode symbols for Greek letters in code
phi_1,phi_2,phi_3,phi_4,sigma, andnushould use Unicode symbolsϕ_1,ϕ_2,ϕ_3,ϕ_4,σ, andν.ϕ_1, ϕ_2, ϕ_3, ϕ_4 = 0.5, -0.2, 0, 0.5 σ = 0.01 ν = 0.01 # Growth rate...ϕ_1, ϕ_2, ϕ_3, ϕ_4 = 0.5, -0.2, 0, 0.5 σ = 0.01 ν = 0.01 # Growth rate...qe-code-002 - Use Unicode symbols for Greek letters in code
AMF_LSS_VARclass definition (which is a duplicate), the variablenuis used instead ofνin the constructor parameter.Unpack required elements
self.A, self.B...`Unpack required elements
self.A, self.B...`νcorrectly in the constructor. This is compliant with qe-code-002.qe-code-002 - Use Unicode symbols for Greek letters in code
AMF_LSS_VARclass. While both versions use Unicode symbols correctly for Greek letters, having duplicate class definitions is problematic for code organization and maintainability. The second, simpler version appears to be for scalar cases only and should either be removed or clearly distinguished as a separate class.class AMF_LSS_VAR: """ This class is written to transform a scalar additive functional i...AMF_LSS_VARclass defined earlier that handles both scalar and vector cases. The duplication creates confusion and potential maintenance issues. The text before this code cell ("Let's write a program to simulate sample paths...") suggests this was meant to introduce the class, but it's already been defined and used extensively earlier in the lecture.Jax (2 issues)
AMF_LSS_VARclass is used for storing model parameters (A, B, D, F, ν) which should be replaced with a NamedTuple. The class contains mutable model parameters and lacks proper separation between data storage and computation logic.class AMF_LSS_VAR: """ This class transforms an additive (multiplicative) functional int...import jax.numpy as jnp
class AMFParams(NamedTuple):
"""Parameter...`
AMF_LSS_VARclass mixes parameter storage with computation logic. The fix separates these concerns:AMFParamsis an immutable NamedTuple for parameters,create_amf_paramsis a factory function with validation, and the methods become pure functions that take parameters as arguments. This follows JAX best practices for functional programming and makes the code more maintainable and testable.AMF_LSS_VARclass definition (scalar version) also violates the rule by using a class for parameter storage instead of a NamedTuple with factory functions.class AMF_LSS_VAR: """ This class is written to transform a scalar additive functional i...import jax.numpy as jnp
class AMFScalarParams(NamedTuple):
"""Par...`
AMFScalarParams) for the scalar case, provide a factory function (create_amf_scalar_params) for validation, and convert methods to pure functions. This maintains consistency with the first fix and follows JAX/QuantEcon style guide requirements for functional programming patterns.Link (5 issues)
qe-link-002 - Use doc links for cross-series references
A convenient way to represent our additive functional is to use a [linear state space system](https:...A convenient way to represent our additive functional is to use a {doc}linear state space system <i...`qe-link-002 - Use doc links for cross-series references
To study it, we could map it into an instance of [LinearStateSpace](https://github.com/QuantEcon/Qua...To study it, we could map it into an instance ofLinearStateSpacefrom QuantEcon.py....qe-link-002 - Use doc links for cross-series references
* For an example of such a mapping, see [this example](https://python.quantecon.org/linear_models.ht...* For an example of such a mapping, see {doc}this example <advanced:linear_models.html#second-order...`qe-link-002 - Use doc links for cross-series references
A convenient way to do this is to construct an appropriate instance of a [linear state space system]...A convenient way to do this is to construct an appropriate instance of a {doc}linear state space sy...`qe-link-002 - Use doc links for cross-series references
This will allow us to use the routines in [LinearStateSpace](https://github.com/QuantEcon/QuantEcon....This will allow us to use the routines inLinearStateSpaceto study dynamics....Math (3 issues)
$\alpha$,$\beta$, etc. for simple parameter mentions in narrative text where no mathematical expressions are present. Should use unicode characters instead for better readability.x_t is an n × 1 vector,
A is an n × n stable matrix (all eigenvalues lie within the open u...`
qe-math-002 - Use \top for transpose notation
\top.$$ \begin{bmatrix} x_t \\ y_t \end{bmatrix} = \begin{bmatrix} 0 & I & 0 \\ 0 & 0 ...$$ \begin{bmatrix} x_t \\ y_t \end{bmatrix} = \begin{bmatrix} 0 & I & 0 \\ 0 & 0 ...qe-math-004 - Do not use bold face for matrices or vectors
* $z_{t+1} \sim {\cal N}(0,I)$ is an $m \times 1$ IID shock,...* $z_{t+1} \sim {\cal N}(0,I)$ is an $m \times 1$ IID shock, where $I$ is the identity matrix,...Writing (34 issues)
qe-writing-001 - Use one sentence per paragraph
Many economic time series display persistent growth that prevents them from being asymptotically st...Many economic time series display persistent growth that prevents them from being asymptotically st...qe-writing-001 - Use one sentence per paragraph
Asymptotic stationarity and ergodicity are key assumptions needed to make it possible to learn by ap...Asymptotic stationarity and ergodicity are key assumptions needed to make it possible to learn by ap...qe-writing-001 - Use one sentence per paragraph
If a process${y_t}$ is an additive functio...`
If a process${y_t}$ is an additive functio...`
{cite}Hansen_2012_Eca` describes a general class of additive functionals.This lecture focuses on ...`
{cite}Hansen_2012_Eca` describes a general class of additive functionals.This lecture focuses on ...`
qe-writing-001 - Use one sentence per paragraph
A convenient way to represent our additive functional is to use a [linear state space system](https:...A convenient way to represent our additive functional is to use a [linear state space system](https:...qe-writing-001 - Use one sentence per paragraph
To study it, we could map it into an instance of [LinearStateSpace](https://github.com/QuantEcon/Qua...To study it, we could map it into an instance of [LinearStateSpace](https://github.com/QuantEcon/Qua...qe-writing-001 - Use one sentence per paragraph
(addfunc_eg1)=
In doing so we'll assume that $z_{t+1...`
(addfunc_eg1)=
In doing so we'll assume that $z_{t+1...`
Let the increment in${y_t}$ obey...`
Let the increment in${y_t}$ obey...`
qe-writing-001 - Use one sentence per paragraph
While {eq}ftafis not a first order system like {eq}old1_additive_functionals, we know that it c...While {eq}ftafis not a first order system like {eq}old1_additive_functionals, we know that it c...qe-writing-001 - Use one sentence per paragraph
This system also constructs the compon...`
This system also constructs the compon...`
Hansen and Sargent {cite}Hans_Sarg_bookdescribe how to construct a decomposition of an additive f...Hansen and Sargent {cite}Hans_Sarg_bookdescribe how to construct a decomposition of an additive f...At this stage, you should pause and verify that $y_{t+1} - y_t$ satisfies {eq}old2_additive_functio...`At this stage, you should pause and verify that $y_{t+1} - y_t$ satisfies {eq}old2_additive_functio...`A convenie...`
A convenie...`
The classAMF_LSS_VARmentioned {ref}above <amf_lss>does all that we want to study our additive...The classAMF_LSS_VARmentioned {ref}above <amf_lss>does all that we want to study our additive...When we plot multiple realizations of a component in the 2nd, 3rd, and 4th panels, we also plot the ...When we plot multiple realizations of a component in the 2nd, 3rd, and 4th panels, we also plot the ...As mentioned above, the process ...`
As mentioned above, the process ...`
An instance of classAMF_LSS_VAR({ref}above <amf_lss>) includes this associated multiplicative...An instance of classAMF_LSS_VAR({ref}above <amf_lss>) includes this associated multiplicative...As before, when we plotted multiple realizations of a component in the 2nd, 3rd, and 4th panels, we ...As before, when we plotted multiple realizations of a component in the 2nd, 3rd, and 4th panels, we ...The top right panel of the above graph shows a panel of martingales associated with the panel of $M_...The top right panel of the above graph shows a panel of martingales associated with the panel of $M_...Hansen and Sargent {cite}Hans_Sarg_book(ch. 8) describe the following two properties of the mart...Hansen and Sargent {cite}Hans_Sarg_book(ch. 8) describe the following two properties of the mart...It remains con...`
It remains con...`
Let's drill down and study probability distribution of the multiplicative martingale $\{\widetilde ...Let's drill down and study probability distribution of the multiplicative martingale $\{\widetilde ......`
...`
We'll do this ...`
We'll do this ...`
The heavy lifting is done inside theAMF_LSS_VAR` class.The following code adds some simple funct...`
The heavy lifting is done inside theAMF_LSS_VAR` class.The following code adds some simple funct...`
Now that we have these functions in our toolkit, let's apply them to run some simulations....Now that we have these functions in our toolkit, let's apply them to run some simulations....Let's plot the probability density functions for $\log {\widetilde M}_t$ for $t=100, 500, 1000, 1000...Let's plot the probability density functions for $\log {\widetilde M}_t$ for $t=100, 500, 1000, 1000...These probability density functions help us understand mechanics underlying the **peculiar property...These probability density functions help us understand mechanics underlying the **peculiar property...Our special additive functional displays interesting time series behavior while also being easy to c...Our special additive functional displays interesting time series behavior while also being easy to c...- $m_t = \sum_{j=1}^t H z_j$, a martingale with time $t+1$ increment $H z_{t+1}$...- $m_t = \sum_{j=1}^t H z_j$, a martingale with time $t+1$ increment $H z_{t+1}$...## A particular additive functional...## A particular additive functional...### Linear state-space representation...### Linear state-space representation...### Associated multiplicative functional...### Associated multiplicative functional...📌 Summary
Found 51 issues across 8 semantic groups
🤖 This PR was automatically generated by the QuantEcon Style Guide Checker
📚 Review the Style Guide Documentation for more details