[arellano] get rid of jitclass #131

shlff · 2023-10-18T03:39:34Z

Hi @jstac this PR gets rid of class and uses namedtuple, as mentioned in #129 .

netlify · 2023-10-18T03:39:43Z

✅ Deploy Preview for incomparable-parfait-2417f8 ready!

Name	Link
🔨 Latest commit	`a3723f0`
🔍 Latest deploy log	https://app.netlify.com/sites/incomparable-parfait-2417f8/deploys/65e6de878f4e0f00085860c4
😎 Deploy Preview	https://deploy-preview-131--incomparable-parfait-2417f8.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2023-10-18T03:50:43Z

🚀 Deployed on https://65e6e110f6b8b87564ec4997--incomparable-parfait-2417f8.netlify.app

jstac · 2024-03-03T20:47:44Z

@shlff , this is labeled as "in-work". Is it ready to review?

shlff · 2024-03-04T00:14:53Z

Thanks @jstac . I've been reviewing this PR, and I will get back to you very soon.

shlff · 2024-03-04T01:24:44Z

Thanks @jstac . This PR is ready to review. Here is a preview:

https://65e521825ca446fa847b6ccd--incomparable-parfait-2417f8.netlify.app/arellano

In addition to using namedtuple, I also modified the words previously mentioning the class and corrected some typos.

jstac · 2024-03-04T01:55:42Z

Thanks @shlff , appreciated.

@HumphreyYang , can you please review?

HumphreyYang

Many thanks @shlff for the careful changes. Please find my minor comments below.

HumphreyYang · 2024-03-04T10:44:25Z

lectures/arellano.md

+def create_arellano(B_size=251,   # Grid size for bonds
+                    B_min=-0.45,   # Smallest B value
+                    B_max=0.45,    # Largest B value
+                    y_size=51,     # Grid size for income
+                    β=0.953,            # Time discount parameter
+                    γ=2.0,              # Utility parameter
+                    r=0.017,            # Lending rate
+                    ρ=0.945,            # Persistence in the income process
+                    η=0.025,            # Standard deviation of the income process
+                    θ=0.282,            # Prob of re-entering financial markets
+                    def_y_param=0.969): # Parameter governing income in default


I think we prefer to comment on parameters when they are defined for the first time. Please move these comments on parameters to where we define the namedtuple.

Thanks @HumphreyYang , I will add the comments to both places, given the comments from another PR.

HumphreyYang · 2024-03-04T10:47:32Z

lectures/arellano.md

 probabilities.

 ```{code-cell} ipython3
-:hide-output: false
+Arellano_Economy = namedtuple('Arellano_Economy', ('β', 'γ', 'r', 'ρ', 'η', 'θ', \


Class (namedtuple) names should follow CamelCase

Suggested change

Arellano_Economy = namedtuple('Arellano_Economy', ('β', 'γ', 'r', 'ρ', 'η', 'θ', \

ArellanoEconomy = namedtuple('Arellano_Economy', ('β', 'γ', 'r', 'ρ', 'η', 'θ', \

There are some Arellano_Economy below in the text as well, so please update them together : )

Good idea! We should change the name in the original lecture too:

https://python-advanced.quantecon.org/arellano.html

HumphreyYang · 2024-03-04T10:52:03Z

lectures/arellano.md

+Arellano_Economy = namedtuple('Arellano_Economy', ('β', 'γ', 'r', 'ρ', 'η', 'θ', \
+                                                  'B_size', 'y_size', \
+                                                  'P', 'B_grid', 'y_grid', 'def_y'))


As noted above, we can migrate the comments below to here.

(Also, we can change lines without \ when they are in the same ())

HumphreyYang · 2024-03-04T10:56:02Z

lectures/arellano.md


 For example, `r=0.017` matches the average quarterly rate on a 5 year US treasury over the period 1983–2001.

 Details on how to compute the figures are reported as solutions to the
 exercises.

 The first figure shows the bond price schedule and replicates Figure 3 of
-Arellano, where $ y_L $ and $ Y_H $ are particular below average and above average
+{cite}`Are08`, where $ y_L $ and $ Y_H $ are particular below average and above average
 values of output $ y $.

 ![https://python-advanced.quantecon.org/_static/lecture_specific/arellano/arellano_bond_prices.png](https://python-advanced.quantecon.org/_static/lecture_specific/arellano/arellano_bond_prices.png)


Should we move these images to this repo as we will migrate the advanced series to theme-specific series soon? (CC @mmcky):

![https://python-advanced.quantecon.org/_static/lecture_specific/arellano/arellano_bond_prices.png](https://python-advanced.quantecon.org/_static/lecture_specific/arellano/arellano_bond_prices.png)

Thanks @HumphreyYang . I reckon @mmcky have already done that in PR #141

mmcky · 2024-03-04T22:32:33Z

@shlff do you know why you needed to use a force push. They are pretty dangerous and I encourage you to avoid using them. Would you like to update this PR from the main as I fixed the image issues (as you commented above).

shlff · 2024-03-04T22:39:34Z

Sorry @mmcky I realised the risk of force push and will not use it anymore.

Yep it would be great if we can update this PR from the main. I reckon that after that some check failure should be gone.

shlff · 2024-03-04T22:43:18Z

Thanks @HumphreyYang @jstac and @mmcky . I resolved the issues mentioned in the comments above.

This PR is ready to review again and here is a preview:

https://65e64dd91341d614052e79ae--incomparable-parfait-2417f8.netlify.app/

jstac · 2024-03-05T05:13:00Z

Many thanks @shlff

Camel case has no spaces, so Arellano_Economy -> ArellanoEconomy
PEP8 should apply as much as possible -- in particular, max 80 characters per line
The following lines are unnecessary because the arrays are already JAX arrays

    B_grid = jax.device_put(B_grid)
    y_grid = jax.device_put(y_grid)

shlff · 2024-03-05T05:18:14Z

Thanks @jstac .

It seems that the commit with changes I made in the morning has not been pushed to this PR.

I will make the changes suggested by your comments along with those changes.

…n/lecture-jax into use_namedtuple_arellano

shlff · 2024-03-05T06:36:18Z

Thanks @jstac . The following tasks have been done:

Camel case has no spaces, so Arellano_Economy -> ArellanoEconomy
PEP8 should apply as much as possible -- in particular, max 80 characters per line
The following lines are unnecessary because the arrays are already JAX arrays

This PR is ready to review again, and please find a preview:

https://65e6b96ed004475661581086--incomparable-parfait-2417f8.netlify.app/arellano

jstac · 2024-03-05T06:57:13Z

Hi @shlff , the first code block (ArellanoEconomy = ...) has lines more than 80 characters. Same with the next one.

shlff · 2024-03-05T07:08:26Z

Good catch @jstac . I modified all the code cells, which should all strictly follow the max 80 characters rule except for line 601.

Please find the preview here:

https://65e6c6b323165e623b58b2dd--incomparable-parfait-2417f8.netlify.app/arellano

jstac · 2024-03-05T07:46:23Z

I'm confused @shlff . In that preview there are multiple lines that exceed 80 characters, even in the first code block.

shlff · 2024-03-05T07:53:39Z

Thanks @jstac . I set a ruler of max 80 characters in my vs code, and here is a screenshot of these two code cells:

I guess the code cell length in the preview is less than 80 characters.

Maybe I can further modify the code so that each line of the code can be revealed without scrolling in the preview.

jstac · 2024-03-05T08:06:05Z

That's strange. Try counting the characters in this line, which i copy pasted from the first code block:

                              'P',       # Markov matrix governing the income process

shlff · 2024-03-05T08:26:44Z

Thanks @jstac . I figured out:

I guess you could review it using an old preview.

If you review the same line from the most recent preview below, then this line should be 80 characters:

https://65e6c6b323165e623b58b2dd--incomparable-parfait-2417f8.netlify.app/arellano

jstac · 2024-03-05T09:11:09Z

Thanks @shlff

I've readjusted those lines so the reader doesn't need to scroll and the comments are aligned.

[arellano] use namedtuple

2fed0fb

shlff added the in-work label Oct 18, 2023

github-actions bot temporarily deployed to pull request October 18, 2023 03:50 Inactive

further_edits

8a1f149

github-actions bot temporarily deployed to pull request March 4, 2024 01:19 Inactive

shlff added ready and removed in-work labels Mar 4, 2024

mmcky requested a review from HumphreyYang March 4, 2024 03:49

HumphreyYang reviewed Mar 4, 2024

View reviewed changes

shlff added 3 commits March 5, 2024 08:51

fix

762521e

[arellano] use namedtuple

f0a5874

further_edits

ff86e0f

shlff force-pushed the use_namedtuple_arellano branch from 762521e to ff86e0f Compare March 4, 2024 22:30

github-actions bot temporarily deployed to pull request March 4, 2024 22:40 Inactive

shlff changed the title ~~[arellano] use namedtuple~~ [arellano] get rid of jitclass Mar 4, 2024

shlff added 3 commits March 5, 2024 16:39

Merge branch 'use_namedtuple_arellano' of https://github.com/QuantEco…

520add0

…n/lecture-jax into use_namedtuple_arellano

Merge branch 'use_namedtuple_arellano' of https://github.com/QuantEco…

f4af18e

…n/lecture-jax into use_namedtuple_arellano

tidyup

3a3d23a

github-actions bot temporarily deployed to pull request March 5, 2024 06:00 Inactive

further_tidyup

90bf9c7

github-actions bot temporarily deployed to pull request March 5, 2024 06:19 Inactive

shlff added 2 commits March 5, 2024 18:02

edits

0dfa2b3

minor_edits

cf734ef

github-actions bot temporarily deployed to pull request March 5, 2024 07:16 Inactive

misc

a3723f0

github-actions bot temporarily deployed to pull request March 5, 2024 09:08 Inactive

jstac merged commit 1cd0306 into main Mar 5, 2024
7 checks passed

jstac deleted the use_namedtuple_arellano branch March 5, 2024 09:11

mmcky mentioned this pull request Mar 7, 2024

[arellano] use namedtuple #129

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[arellano] get rid of jitclass #131

[arellano] get rid of jitclass #131

shlff commented Oct 18, 2023 •

edited

Loading

netlify bot commented Oct 18, 2023 •

edited

Loading

github-actions bot commented Oct 18, 2023 •

edited

Loading

jstac commented Mar 3, 2024

shlff commented Mar 4, 2024

shlff commented Mar 4, 2024

jstac commented Mar 4, 2024

HumphreyYang left a comment •

edited

Loading

HumphreyYang Mar 4, 2024 •

edited

Loading

shlff Mar 4, 2024

HumphreyYang Mar 4, 2024

shlff Mar 4, 2024

HumphreyYang Mar 4, 2024 •

edited

Loading

HumphreyYang Mar 4, 2024 •

edited

Loading

shlff Mar 4, 2024 •

edited

Loading

mmcky commented Mar 4, 2024

shlff commented Mar 4, 2024

shlff commented Mar 4, 2024

jstac commented Mar 5, 2024

shlff commented Mar 5, 2024

shlff commented Mar 5, 2024

jstac commented Mar 5, 2024

shlff commented Mar 5, 2024 •

edited

Loading

jstac commented Mar 5, 2024

shlff commented Mar 5, 2024

jstac commented Mar 5, 2024 •

edited

Loading

shlff commented Mar 5, 2024 •

edited

Loading

jstac commented Mar 5, 2024

	Arellano_Economy = namedtuple('Arellano_Economy', ('β', 'γ', 'r', 'ρ', 'η', 'θ', \
	ArellanoEconomy = namedtuple('Arellano_Economy', ('β', 'γ', 'r', 'ρ', 'η', 'θ', \

[arellano] get rid of jitclass #131

[arellano] get rid of jitclass #131

Conversation

shlff commented Oct 18, 2023 • edited Loading

netlify bot commented Oct 18, 2023 • edited Loading

✅ Deploy Preview for incomparable-parfait-2417f8 ready!

github-actions bot commented Oct 18, 2023 • edited Loading

jstac commented Mar 3, 2024

shlff commented Mar 4, 2024

shlff commented Mar 4, 2024

jstac commented Mar 4, 2024

HumphreyYang left a comment • edited Loading

Choose a reason for hiding this comment

HumphreyYang Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

shlff Mar 4, 2024

Choose a reason for hiding this comment

HumphreyYang Mar 4, 2024

Choose a reason for hiding this comment

shlff Mar 4, 2024

Choose a reason for hiding this comment

HumphreyYang Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

HumphreyYang Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

shlff Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

mmcky commented Mar 4, 2024

shlff commented Mar 4, 2024

shlff commented Mar 4, 2024

jstac commented Mar 5, 2024

shlff commented Mar 5, 2024

shlff commented Mar 5, 2024

jstac commented Mar 5, 2024

shlff commented Mar 5, 2024 • edited Loading

jstac commented Mar 5, 2024

shlff commented Mar 5, 2024

jstac commented Mar 5, 2024 • edited Loading

shlff commented Mar 5, 2024 • edited Loading

jstac commented Mar 5, 2024

shlff commented Oct 18, 2023 •

edited

Loading

netlify bot commented Oct 18, 2023 •

edited

Loading

github-actions bot commented Oct 18, 2023 •

edited

Loading

HumphreyYang left a comment •

edited

Loading

HumphreyYang Mar 4, 2024 •

edited

Loading

HumphreyYang Mar 4, 2024 •

edited

Loading

HumphreyYang Mar 4, 2024 •

edited

Loading

shlff Mar 4, 2024 •

edited

Loading

shlff commented Mar 5, 2024 •

edited

Loading

jstac commented Mar 5, 2024 •

edited

Loading

shlff commented Mar 5, 2024 •

edited

Loading