Skip to content

Conversation

bishmaybarik
Copy link
Member

@bishmaybarik bishmaybarik commented Aug 18, 2025

This PR brings the following changes to the lecture:

  • Fixes [samuelson] Incorrect mathematical equation #530 : I have rectified the error in the mathematical equation
  • Fixes [samuelson] Inconsistent usage of ($a$, $b$) and ($\alpha$, $\beta$) #456 : I have updated the lectures to use $\alpha$ and $\beta$ throughout; and this is consistent with the explanation on text as well
  • Fixes [samuelson] Does not follow Quantecon style guidelines #531 : Making some more changes to follow John's suggestions
    • large code cells with lots of plotting code are typically hidden now -- see, e.g., def param_plot()
    • we might want to truncate the number of decimal points in output such as (1.5371322893124, -0.9024999999999999)
    • we shouldn't have comments in cells, such as ### Test the categorize_solution function -- if necessary, those comments should be ordinary markdown above the cells.
    • I don't think we should have plt.rcParams["figure.figsize"] = (11, 5) #set default figure size
    • the align syntax after "Notice that" and in other places seems nonstandard

Additional tasks

  • run black over code-cells for PEP8 / code style compliance (line-length=80)
  • adjusted bold to emphasis for non-definitions in text (style-guide)

This commit rectifies the notations and replaces the greek letters (alpha and beta) with (a and b)
Copy link

github-actions bot commented Aug 18, 2025

@github-actions github-actions bot temporarily deployed to pull request August 18, 2025 15:16 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 18, 2025 15:17 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 20, 2025 06:08 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 20, 2025 06:08 Inactive
@mmcky mmcky added lecture and removed content labels Aug 21, 2025
@bishmaybarik
Copy link
Member Author

I am not sure if I should completely delete the following three lines of code:

### code to reverse-engineer a cycle
### y_t = r^t (c_1 cos(ϕ t) + c2 sin(ϕ t))
###

from this entire code chunk (it can be found here):

### code to reverse-engineer a cycle
### y_t = r^t (c_1 cos(ϕ t) + c2 sin(ϕ t))
###

def f(r, ϕ):
    """
    Takes modulus r and angle ϕ of complex number r exp(j ϕ)
    and creates ρ1 and ρ2 of characteristic polynomial for which
    r exp(j ϕ) and r exp(- j ϕ) are complex roots.

    Returns the multiplier coefficient a and the accelerator coefficient b
    that verifies those roots.
    """
    g1 = cmath.rect(r, ϕ)  # Generate two complex roots
    g2 = cmath.rect(r, -ϕ)
    ρ1 = g1 + g2           # Implied ρ1, ρ2
    ρ2 = -g1 * g2
    b = -ρ2                # Reverse-engineer a and b that validate these
    a = ρ1 - b
    return ρ1, ρ2, a, b

## Now let's use the function in an example
## Here are the example parameters

r = .95
period = 10                # Length of cycle in units of time
ϕ = 2 * math.pi/period

## Apply the function

ρ1, ρ2, a, b = f(r, ϕ)

print(f"a, b = {a}, {b}")
print(f"ρ1, ρ2 = {ρ1}, {ρ2}")

Also, there are comments inside the code cell, like:

## Now let's use the function in an example
## Here are the example parameters

which I think might be a little confusing if people miss out reading each line of the code.


I plan to rewrite the entire thing like this:

def f(r, ϕ):
    """
    Takes modulus r and angle ϕ of complex number r exp(j ϕ)
    and creates ρ1 and ρ2 of characteristic polynomial for which
    r exp(j ϕ) and r exp(- j ϕ) are complex roots.

    Returns the multiplier coefficient a and the accelerator coefficient b
    that verifies those roots.
    """
    g1 = cmath.rect(r, ϕ)  # Generate two complex roots
    g2 = cmath.rect(r, -ϕ)
    ρ1 = g1 + g2           # Implied ρ1, ρ2
    ρ2 = -g1 * g2
    b = -ρ2                # Reverse-engineer a and b that validate these
    a = ρ1 - b
    return ρ1, ρ2, a, b

Now let's use the function in an example. Here are the example parameters:

r = .95
period = 10                # Length of cycle in units of time
ϕ = 2 * math.pi/period

## Apply the function
ρ1, ρ2, a, b = f(r, ϕ)

print(f"a, b = {a}, {b}")
print(f"ρ1, ρ2 = {ρ1}, {ρ2}")

Please let me know your thoughts on this @HumphreyYang and @mmcky. Once you give your inputs, I can make the changes accordingly.

@HumphreyYang
Copy link
Member

I am not sure if I should completely delete the following three lines of code:

### code to reverse-engineer a cycle
### y_t = r^t (c_1 cos(ϕ t) + c2 sin(ϕ t))
###

from this entire code chunk (it can be found here):

### code to reverse-engineer a cycle
### y_t = r^t (c_1 cos(ϕ t) + c2 sin(ϕ t))
###

def f(r, ϕ):
    """
    Takes modulus r and angle ϕ of complex number r exp(j ϕ)
    and creates ρ1 and ρ2 of characteristic polynomial for which
    r exp(j ϕ) and r exp(- j ϕ) are complex roots.

    Returns the multiplier coefficient a and the accelerator coefficient b
    that verifies those roots.
    """
    g1 = cmath.rect(r, ϕ)  # Generate two complex roots
    g2 = cmath.rect(r, -ϕ)
    ρ1 = g1 + g2           # Implied ρ1, ρ2
    ρ2 = -g1 * g2
    b = -ρ2                # Reverse-engineer a and b that validate these
    a = ρ1 - b
    return ρ1, ρ2, a, b

## Now let's use the function in an example
## Here are the example parameters

r = .95
period = 10                # Length of cycle in units of time
ϕ = 2 * math.pi/period

## Apply the function

ρ1, ρ2, a, b = f(r, ϕ)

print(f"a, b = {a}, {b}")
print(f"ρ1, ρ2 = {ρ1}, {ρ2}")

Also, there are comments inside the code cell, like:

## Now let's use the function in an example
## Here are the example parameters

which I think might be a little confusing if people miss out reading each line of the code.

I plan to rewrite the entire thing like this:

def f(r, ϕ):
    """
    Takes modulus r and angle ϕ of complex number r exp(j ϕ)
    and creates ρ1 and ρ2 of characteristic polynomial for which
    r exp(j ϕ) and r exp(- j ϕ) are complex roots.

    Returns the multiplier coefficient a and the accelerator coefficient b
    that verifies those roots.
    """
    g1 = cmath.rect(r, ϕ)  # Generate two complex roots
    g2 = cmath.rect(r, -ϕ)
    ρ1 = g1 + g2           # Implied ρ1, ρ2
    ρ2 = -g1 * g2
    b = -ρ2                # Reverse-engineer a and b that validate these
    a = ρ1 - b
    return ρ1, ρ2, a, b

Now let's use the function in an example. Here are the example parameters:

r = .95
period = 10                # Length of cycle in units of time
ϕ = 2 * math.pi/period

## Apply the function
ρ1, ρ2, a, b = f(r, ϕ)

print(f"a, b = {a}, {b}")
print(f"ρ1, ρ2 = {ρ1}, {ρ2}")

Please let me know your thoughts on this @HumphreyYang and @mmcky. Once you give your inputs, I can make the changes accordingly.

Looks great to me! Thanks so much!

@github-actions github-actions bot temporarily deployed to pull request August 21, 2025 06:05 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 21, 2025 06:22 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 21, 2025 09:34 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 21, 2025 09:35 Inactive
@bishmaybarik bishmaybarik marked this pull request as ready for review August 23, 2025 10:42
@github-actions github-actions bot temporarily deployed to pull request August 23, 2025 11:01 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 23, 2025 11:01 Inactive
@HumphreyYang HumphreyYang requested a review from Copilot August 23, 2025 16:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@mmcky mmcky requested a review from Copilot August 25, 2025 04:10
@bishmaybarik bishmaybarik requested a review from mmcky September 9, 2025 06:48

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Copy link

📖 Netlify Preview Ready!

Preview URL: https://pr-561--sunny-cactus-210e3e.netlify.app (2ea34d3)

📚 Changed Lecture Pages: samuelson

Copy link

📖 Netlify Preview Ready!

Preview URL: https://pr-561--sunny-cactus-210e3e.netlify.app (fac1303)

📚 Changed Lecture Pages: samuelson

Copy link

📖 Netlify Preview Ready!

Preview URL: https://pr-561--sunny-cactus-210e3e.netlify.app (509a5de)

📚 Changed Lecture Pages: samuelson

@mmcky
Copy link
Contributor

mmcky commented Sep 12, 2025

@HumphreyYang I have:

  • ran black over code-cells to update the code re: code-styler
  • switched bold to emphasis for non-definitions.

Do you want to do a final check and then we can merge this.

@mmcky mmcky added the ready label Sep 12, 2025
@HumphreyYang
Copy link
Member

Hi @bishmaybarik and @mmcky,

I have made some big changes to make the code in the lecture cleaner in 5c8f323.

Some main changes are:

  • Added analyze_roots and unified simulator simulate_samuelson so we can remove repetition later in the lecture.
  • Simplified categorize_solution logic/messages.
  • Cleaned up plotting.
  • Samuelson class now uses analysis/simulator with legend/param fixes to remove repetition in the old code.
  • Use unicode in the class and reduce the length of code to below 80 characters.

Please let me know what you think!

Copy link

📖 Netlify Preview Ready!

Preview URL: https://pr-561--sunny-cactus-210e3e.netlify.app (5c8f323)

📚 Changed Lecture Pages: samuelson

Copy link

📖 Netlify Preview Ready!

Preview URL: https://pr-561--sunny-cactus-210e3e.netlify.app (d4ad185)

📚 Changed Lecture Pages: samuelson

Copy link

📖 Netlify Preview Ready!

Preview URL: https://pr-561--sunny-cactus-210e3e.netlify.app (c0e6bc1)

📚 Changed Lecture Pages: samuelson

@mmcky
Copy link
Contributor

mmcky commented Sep 15, 2025

thanks @HumphreyYang -- these are big changes and lots of reorganisation.

@bishmaybarik would you mind to read the updated version and let me know your thoughts on these changes?

@mmcky mmcky removed the ready label Sep 15, 2025
@bishmaybarik
Copy link
Member Author

thanks @HumphreyYang -- these are big changes and lots of reorganisation.

@bishmaybarik would you mind to read the updated version and let me know your thoughts on these changes?

Thanks for making such significant changes @HumphreyYang . Sure @mmcky , I am reviewing it right now and will let you know my thoughts on this as soon as possible.

@bishmaybarik
Copy link
Member Author

thanks @HumphreyYang -- these are big changes and lots of reorganisation.
@bishmaybarik would you mind to read the updated version and let me know your thoughts on these changes?

Thanks for making such significant changes @HumphreyYang . Sure @mmcky , I am reviewing it right now and will let you know my thoughts on this as soon as possible.

These are nice changes @HumphreyYang !

@mmcky I have gone through the entire lecture, and these are some of my quick thoughts:

  • The codes are now concise and to the point.
  • Figures look much better. There were some alignment issues in the legends, and Humphrey's version fixes that as well.
  • Reorganisation of some parts of the lecture makes the entire lecture easier to follow.

Additionally, I reviewed the mathematical calculations and verified the results produced by the code. The math appears to be correct, and upon comparing this version with the previous one, I found that the outputs are consistent.

P.S. This is a great way to write codes -- I’m learning a lot from @HumphreyYang!

@mmcky
Copy link
Contributor

mmcky commented Sep 16, 2025

thanks @bishmaybarik for your proof read and review -- it is greatly appreciated.

Thanks @HumphreyYang for your updates and review as well.

@HumphreyYang let me know when you're happy for this to be merged.

@mmcky mmcky added ready and removed in-work labels Sep 16, 2025
@jstac
Copy link
Contributor

jstac commented Sep 19, 2025

Many thanks @bishmaybarik @HumphreyYang . Very nice work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants