Skip to content

Fix transpose typos in additive functional lecture #120

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

Merged
merged 1 commit into from
May 7, 2020

Conversation

jmbejara
Copy link
Contributor

@jmbejara jmbejara commented May 5, 2020

It looks like there are some typos in the formulas for the decomposition. Matrices don't conform. I've fixed them in this PR. Hope this looks ok. Great lectures, btw!

@jstac
Copy link
Contributor

jstac commented May 6, 2020

Thanks @jmbejara ! All feedback & PRs are appreciated.

@shizejin, might you be able to review this or request someone else?

@shizejin
Copy link
Member

shizejin commented May 6, 2020

@jstac Sure I would love to review this. I'll do this today.

@shizejin
Copy link
Member

shizejin commented May 6, 2020

Hi @jmbejara, thank you so much for spotting these typos. It is true that the formulas for H and g matrices are not correct. Specifically, I observed inconsistency of shapes of D, F, H, and g matrices in the lecture, which I think is what causes the problem that you very sharply pointed out.

If we stick to the dimensions of D and F as in equation (2), then the modifications you made is correct. Here I derived H and g matrices with details.

It turns out that there are more corrections to make in addition to what you pointed out:

It would be great to have your opinions about how to complete this correction! @jstac @jmbejara

@jmbejara
Copy link
Contributor Author

jmbejara commented May 6, 2020

@shizejin Ok. You're right. I missed those. I've made those additional changes and amended the original commit. I also made one more change to line 1044 (changing A' to A). Thanks!

@jstac
Copy link
Contributor

jstac commented May 6, 2020

Thanks guys! Looks good. @mmcky, I see this is failing due to checking out of QE theme. What should we do here?

@mmcky
Copy link
Contributor

mmcky commented May 7, 2020

@jstac after searching and doing some debug work (i.e. reseting security keys etc) it has dawned on me what the issue is. It will need a bit more work to solve so I will raise an issue. Please go ahead and merge -- if you would like an html preview I can switch off the theme step.

The issue here is when the theme files are fetched from a private repo -- the commit author is @jmbejara and a 404 is returned from GitHub.

@jstac
Copy link
Contributor

jstac commented May 7, 2020

Roger that @mmcky and thanks all. This is greatly appreciated.

@jstac jstac merged commit d4022ca into QuantEcon:master May 7, 2020
@jmbejara jmbejara deleted the fix_typo branch May 7, 2020 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants