Address comments for cagan_ree lecture

[HumphreyYang]

This PR addresses items related to code and minor typos in #386.

[netlify]

✅ Deploy Preview for taupe-gaufre-c4e660 ready!

Name	Link
🔨 Latest commit	44a47ff
🔍 Latest deploy log	https://app.netlify.com/sites/taupe-gaufre-c4e660/deploys/660cba8921bb390008e5823e
😎 Deploy Preview	https://deploy-preview-387--taupe-gaufre-c4e660.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]

🚀 Deployed on https://660cbc78bb7e396771e0b6e9--taupe-gaufre-c4e660.netlify.app

[mmcky]

thanks @HumphreyYang I have added a few minor comments.

Also I note that there are still some open checkboxes in #386.

Do we need to wait until all those checkboxes are addressed?

[HumphreyYang]

Do we need to wait until all those checkboxes are addressed?

Many thanks @mmcky, I think those unticked boxes are ones we are not sure if we should fix before consulting Tom.

[HumphreyYang]

Hi @mmcky,

I think removing figure size is probably not a good idea. I will revert this change : )

[mmcky]

@HumphreyYang can you post a screen shot. We may be able to use scale instead in myst rather than the code

[mmcky]

@HumphreyYang oh I see. This is actually an aspect ratio issue. This might be a good use case for specifying fig size. I agree.

cc @jstac

[jstac]

Thanks @HumphreyYang . Just one comment above.

[jstac]

Thanks for this @HumphreyYang . Is it still in-work?

[HumphreyYang]

Thanks for this @HumphreyYang . Is it still in-work?

Many thanks @jstac, there are some items that have not been addressed in #386, which is now confirmed by Tom. I will address the remaining items this week.

[HumphreyYang]

Many thanks @mmcky, I will quickly optimize the code for graphs and then it will be ready.

[HumphreyYang]

Hi @mmcky, I intentionally kept the old easy plotting function although my new plotting function experiment_plot can cover the easy cases. The rationale is that I want to hide the complexity in the graphing config (this is why they are hidden) and show the clean and easy plotting function solve_and_plot.

Please let me know your thoughts on this.

[jstac]

Nice work @HumphreyYang . Minor comments above.

[mmcky]

thanks @HumphreyYang 44a47ff

[jstac]

Thanks guys. There are some unchecked boxes in #386. Are they completed?

@mmcky Please merge this when ready.

[HumphreyYang]

Thanks guys. There are some unchecked boxes in #386. Are they completed?

Hi @jstac, the unchecked boxes are related to the explanations in the main text. Would you like me to attempt to address them before passing them to Tom?

[jstac]

Thanks @HumphreyYang . I'm going to leave this one for you and @mmcky and @thomassargent30 to coordinate.

[HumphreyYang]

Many thanks @jstac.

Hi @mmcky, should we merge this first PR first while keeping the issue open? I think Tom is making some changes in the main branch later today.

[mmcky]

thanks @HumphreyYang I will coordinate with @thomassargent30

[mmcky]

Hi @jstac, the unchecked boxes are related to the explanations in the main text. Would you like me to attempt to address them before passing them to Tom?

@HumphreyYang I am happy to merge this as is and you can iterate on the additional check boxes. However please update the top level comment to provide a brief (one sentence description of the changes targetted int his PR and remove the resolves as merging this will close that issue). Thanks.

[HumphreyYang]

@HumphreyYang I am happy to merge this as is and you can iterate on the additional check boxes. However please update the top level comment to provide a brief (one sentence description of the changes targetted int his PR and remove the resolves as merging this will close that issue). Thanks.

Many thanks @mmcky. I have disassociated this PR from the issue.