-
Notifications
You must be signed in to change notification settings - Fork 17
Improve and restructure plotting documentation for better organization and readability #170
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
base: combine_plots
Are you sure you want to change the base?
Conversation
- Simplified complex sentences while preserving technical information - Enhanced transitions between sections for better flow - Improved consistency in tone throughout the document - Clarified explanations and removed redundant phrasing - Fixed grammatical issues and improved sentence structure - Made the text more accessible without losing technical depth Co-authored-by: thorek1 <[email protected]>
Co-authored-by: thorek1 <[email protected]>
thorek1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot avoid using 'we' or 'you'
docs/src/plotting.md
Outdated
|  | ||
|
|
||
| The steady states and dynamics changed as a result of changing the discount factor. As it is a bit more difficult to see what changed between the previous IRF with β = 0.99 and the current one with β = 0.95, we can overlay the two IRFs. Since parameter changes are permanent we first must first set β = 0.99 again and then overlay the IRF with β = 0.95 on top of it: | ||
| The steady states and dynamics changed as a result of changing the discount factor. To better visualize the differences between β = 0.99 and β = 0.95, we can overlay the two IRFs. Since parameter changes are permanent, we must first reset β = 0.99 before overlaying the IRF with β = 0.95 on top of it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot put quotation marks around these parts of the text: β = 0.99
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added quotation marks around "β = 0.99" and "β = 0.95" at line 751. (b1bed52)
docs/src/plotting.md
Outdated
|  | ||
|
|
||
| c and P appear twice, once as the variable itself and once as an auxiliary variable with the L(1) superscript, indicating that it is the value of the variable in t+1 as it is expected to be in t. | ||
| Both c and P appear twice: once as the variable itself and once as an auxiliary variable with the L(1) superscript, representing the value of the variable in t+1 as expected in t. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot put quotation marks around c, P, L(1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added quotation marks around "c", "P", and "L(1)" at line 664. (b1bed52)
docs/src/plotting.md
Outdated
|  | ||
|
|
||
| The legend below the plot indicates which color corresponds to which value of β and the table underneath the plot shows the relevant steady states for both values of β. Note that the steady states differ across the two values of β and also the dynamics, even when the steady state is still the same (e.g. for Y). | ||
| The legend below the plot indicates which color corresponds to each β value, with the table underneath showing the relevant steady states. Note that both the steady states and dynamics differ across the two β values, even when the steady state remains the same (e.g., for Y). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot put quotations marks around Y and β as those are variables and parameters of the model. apply this change throughout the plotting.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added quotation marks around variable/parameter names throughout the document, including "Y", "β", "C", "S", "A", "P", "nu", "R", "MC", "N", "W_real", "Pi", "eps_a", "eps_z", "eps_nu", and "τ" when they appear in prose text. (b1bed52)
docs/src/plotting.md
Outdated
|
|
||
| ### `plot_attributes` | ||
|
|
||
| [Default: `Dict()`]: dictionary of attributes passed on to the plotting function. See the Plots.jl documentation for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot instead of writing [Default: Dict()] write in text what the default argument is. do this throughout plotting.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced all [Default: ...] formats with prose descriptions throughout plotting.md. For example, "[Default: 6, Type: Int]:" became "... (default: 6)". Applied to 15 parameter descriptions. (b1bed52)
docs/src/plotting.md
Outdated
| All text in the plot is now in the computer modern font. Do note that the rendering of the fonts inherits the constraints of the plotting backend (GR in this case) - e.g. the superscript + is not rendered properly for this font. | ||
| All text in the plot now uses the Computer Modern font. Note that font rendering inherits the constraints of the plotting backend (GR in this case)—for example, the superscript + is not rendered properly for this font. | ||
|
|
||
| ### `plots_per_page` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot instead of using the exact argument name in quotation marks, use a more readable section title and clearly refer to the exact argument name in the following description. apply this change throughout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced all section headers with readable titles and added explicit parameter name references in descriptions. For example, "### algorithm" became "### Solution Algorithm" with description starting "The algorithm parameter specifies...". Applied to all 20 parameter sections throughout the document. (b1bed52)
- Converted to passive voice or imperative mood throughout - Changed 'you can' to 'can be' or imperative constructions - Changed 'we' constructions to passive or neutral forms - Maintained all technical content and examples - Preserved document structure and line count Co-authored-by: thorek1 <[email protected]>
thorek1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot address all comments
- Add quotation marks around variable/parameter names (β, Y, C, P, nu, etc.) - Replace [Default: ...] format with prose descriptions - Replace section headers with readable titles (e.g., `algorithm` → Solution Algorithm) - Add explicit parameter name references in section descriptions - Maintain all technical content and examples Co-authored-by: thorek1 <[email protected]>
- Remove remaining "Let's" constructions for consistency - Fix redundant text in GIRF configuration section - Ensure all variable/parameter mentions have quotes Co-authored-by: thorek1 <[email protected]>
docs/src/plotting.md
Outdated
|  | ||
|
|
||
| In the plots we now see both solution methods overlaid. The first-order solution is shown in blue, the second-order solution in orange, as indicated in the legend below the plot. Note that the steady state levels can be different for the two solution methods. For variables where the relevant steady state (non-stochastic steady state for first order and stochastic steady state for higher order) is the same (e.g. `A`) we see the level on the left axis and percentage deviations on the right axis. For variables where the steady state differs between the two solution methods (e.g. `C`) we only see absolute level deviations (`abs. Δ`) on the left axis. Furthermore, the relevant steady state level is mentioned in a table below the plot for reference (rounded so that you can spot the difference to the nearest comparable steady state). | ||
| The plots now show both solution methods overlaid. The first-order solution is shown in blue, the second-order solution in orange, as indicated in the legend below the plot. Note that the steady state levels can be different for the two solution methods. For variables where the relevant steady state is the same for both methods (e.g., `A`), the level appears on the left axis and percentage deviations on the right axis. For variables where the steady state differs between methods (e.g., "C"), only absolute level deviations (`abs. Δ`) appear on the left axis. The relevant steady state levels are shown in a table below the plot for reference (rounded to help identify differences). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot now C is in quotation marks but I actually meant these: C instead of these "C". apply this change to everything that was previously put in quotation marks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced all regular quotation marks with backticks for variable/parameter names throughout the document. Changed "C" → C, "β" → β, "Y" → Y, etc. String literals in code examples (e.g., shocks = "eps_a") were preserved. (2c1637e)
docs/src/plotting.md
Outdated
| ### Shocks | ||
|
|
||
| Shocks determine which IRFs to calculate. Inputs can be a shock name passed on as either a Symbol or String (e.g. :y, or "y"), or Tuple, Matrix or Vector of String or Symbol. `:simulate` triggers random draws of all shocks (excluding occasionally binding constraints (OBC) related shocks). `:all_excluding_obc` contains all shocks but not the OBC related ones. `:all` also includes the OBC related shocks. A series of shocks can be passed on using either a Matrix{Float64}, or a `KeyedArray{Float64}` as input with shocks (Symbol or String) in rows and periods in columns. The `KeyedArray` type is provided by the AxisKeys package. The period of the simulation will correspond to the length of the input in the period dimension plus the number of periods defined in the `periods` argument. If the series of shocks is input as a `KeyedArray{Float64}` make sure to name the rows with valid shock names of type Symbol. Any shocks not part of the model will trigger a warning. `:none` in combination with an `initial_state` can be used for deterministic simulations. | ||
| The `shocks` parameter determines which IRFs to calculate. Shock names can be specified as either a Symbol or String (e.g. :y, or "y"), or Tuple, Matrix or Vector of String or Symbol. `:simulate` triggers random draws of all shocks (excluding those related to occasionally binding constraints). `:all_excluding_obc` includes all shocks except OBC-related ones. `:all` also includes the OBC related shocks. A series of shocks can be passed using either a `Matrix{Float64}` or a `KeyedArray{Float64}`, with shocks (Symbol or String) in rows and periods in columns. The `KeyedArray` type is provided by the AxisKeys package. The simulation length equals the input length in the period dimension plus the number of periods specified in the `periods` argument. When using a `KeyedArray{Float64}`, ensure that rows are named with valid shock names of type Symbol. Any shocks not part of the model will trigger a warning. `:none` in combination with an `initial_state` can be used for deterministic simulations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot shocks is not a parameter but a function argument. all sections here explain function arguments. make sure they are referred to as such throughout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed all references from "parameter" to "argument" for function arguments throughout the document. Model parameters (β, τ, etc.) remain as "parameters". (2a4f8ab)
docs/src/plotting.md
Outdated
| affects the size of shocks as long as they are not set to :none or a shock matrix. | ||
| [Default: `1.0`, Type: `Real`]: size of the shocks in standard deviations. Only affects shocks that are not passed on as a matrix or `KeyedArray` or set to :none. A negative value will flip the sign of the shock. | ||
| You can set the size of the shock using the `shock_size` argument. Here we set it to -2 standard deviations: | ||
| Affects the size of shocks when they are not set to `:none` or provided as a shock matrix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot here you forgot to mention the name of the argument: shock_size
make sure you mention it throughout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added missing argument name shock_size to the Shock Size section description. (2a4f8ab)
…se replace_indices_in_symbol for improved symbol processing
…_decomposition calls for clarity
…n functions for clearer logging and use verbose = false instead
…nditionally use LinearVerbosity for improved logging
…ved solver robustness
…ion solutions with independent covariance support (#180)
…orek1/MacroModelling.jl into copilot/improve-plotting-docs
…_efficient_order (#181) * Initial plan * Add support for grouped covariance input in get_statistics Co-authored-by: thorek1 <[email protected]> * Add test for grouped covariance functionality Co-authored-by: thorek1 <[email protected]> * Fix nested vector handling for String_input in grouped covariance Co-authored-by: thorek1 <[email protected]> * Add documentation for grouped covariance feature Co-authored-by: thorek1 <[email protected]> * Simplify grouped covariance helper functions based on code review Co-authored-by: thorek1 <[email protected]> * Fix documentation wording for covariance parameter Co-authored-by: thorek1 <[email protected]> * Change grouped covariance to return single matrix with zeros for cross-group elements Co-authored-by: thorek1 <[email protected]> * Optimize higher order propagation code to reduce allocations Replace nested loops with vectorized operations using Kronecker product selectors. Instead of iterating through all column indices and checking conditions in nested loops, generate a selector vector that identifies relevant columns where all states are dependencies, then multiply with the solution matrix once. Co-authored-by: thorek1 <[email protected]> * Further optimize determine_efficient_order to minimize allocations Key optimizations: 1. Precompute repeated indexin and matrix slicing operations outside loops 2. Replace copy(dependencies_in_states) with direct assignment to avoid allocations 3. Vectorize nested loops for second/third order dependency detection using reshape and sum operations 4. Build lookup dictionary for covariance variable searches instead of repeated findfirst calls 5. Reuse precomputed matrix slices in propagation loops 6. Eliminate redundant while loop condition checks Co-authored-by: thorek1 <[email protected]> * Update documentation example to show two variables in each group Changed the grouped covariance example from [[:c, :k], [:q]] to [[:c, :k], [:y, :i]] to better demonstrate the feature with balanced groups. Co-authored-by: thorek1 <[email protected]> * Add support for vector of tuples and tuple of tuples in grouped covariance, simplify indicator check Extended Symbol_input and String_input to support Vector{Tuple} and Tuple{Tuple} formats for grouped covariance. Updated is_grouped_covariance_input, parse_covariance_groups, and parse_variables_input_to_index to handle these new formats. Simplified is_grouped_covar indicator by directly checking !isnothing(covar_groups) where needed. Co-authored-by: thorek1 <[email protected]> * Apply vectorized approach to all remaining nested loops in determine_efficient_order Replaced all remaining nested loops in covariance variable dependency detection with vectorized reshape and sum operations. Also precomputed matrix slices and eliminated copy() calls in propagation loops for both second and third order covariance sections. Co-authored-by: thorek1 <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: thorek1 <[email protected]>
…in functionality tests
…date covariance type to include additional structures
Improve and restructure plotting documentation for better organization and readability
Comprehensive improvements to plotting documentation including enhanced readability, consistent formatting, complete argument documentation, and restructuring into focused files for better navigation.
Major Changes
Documentation Restructuring
plotting.md: General overview and setup (70 lines)irf.md: Complete IRF plotting documentation with wrapper functions (1,280+ lines)plot_solution.md: Restructured to match official help entry with state mapping, pruned solutions, and steady state explanations (400+ lines)plot_conditional_forecast.md: Complete conditional forecast plotting documentation (342 lines)plot_conditional_variance_decomposition.md: Complete FEVD plotting documentation (262 lines)plot_model_estimates.md: Complete model estimation plotting with verified arguments (450+ lines)make.jlto include all six plotting documentation filesContent Improvements
plot_simulation: Wrapper forplot_irfwithshocks = :simulateandperiods = 100plot_girf: Wrapper forplot_irfwithgeneralised_irf = trueTechnical Corrections
Results
Total lines: 2,850+ lines across six files
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.