Skip to content
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

Update get_predictions_from_sims.jl #15

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

SGHoekstra
Copy link
Contributor

I think I found a bug in the get_predictions_from_sims.jl script. The wrong transformed variable was used in the cumprod function which led to negative numbers and domain errors. Before the variable is used in the cumprod function it should be exponentiated just like other variables in the script. This proposed fix makes sure that the right transformed variable is used.

The wrong transformed variable was used in the cumprod function which led to negative numbers and domain errors. This proposed fix makes sure that the right transformed variable is used.
@AldoGl
Copy link
Collaborator

AldoGl commented Oct 9, 2024

Hi @SGHoekstra, thank you so much for your contribution!

I see you have changed

dict["x"] = exp.(x) .- 1
to
x = exp.(x) .- 1
+
dict["x"] = x

I like it as it makes the code more readable. We will merge this for sure, but is there really a bug? In which line?

In general, this script is still experimental. If you manage to improve it further (e.g., by making it more modular) please do not hesitate to open a PR!

@SGHoekstra
Copy link
Contributor Author

Hi @AldoGl ,

This was the old situation:

real_government_consumption = sims.real_government_consumption
real_government_consumption_growth_quarterly = diff(log.(real_government_consumption), dims = 1)
model_dict["real_government_consumption_growth_quarterly"] = exp.(real_government_consumption_growth_quarterly) .- 1
real_government_consumption_quarterly =
data["real_government_consumption_quarterly"][data["quarters_num"] .== quarter_num] .*
cumprod(1 .+ real_government_consumption_growth_quarterly, dims = 1)

This is the new situation:

real_government_consumption = sims.real_government_consumption
real_government_consumption_growth_quarterly = diff(log.(real_government_consumption), dims = 1)
real_government_consumption_growth_quarterly = exp.(real_government_consumption_growth_quarterly) .- 1

model_dict["real_government_consumption_growth_quarterly"] = real_government_consumption_growth_quarterly
real_government_consumption_quarterly =
  data["real_government_consumption_quarterly"][data["quarters_num"] .== quarter_num] .*
  cumprod(1 .+ real_government_consumption_growth_quarterly, dims = 1).

In the old situation the variable real_government_consumption_growth_quarterly was used in the cumprod function before the exponent function was applied. So the exponent was applied to model_dict["real_government_consumption_growth_quarterly"] but not to real_government_consumption_growth_quarterly. So If you pay close attention I made sure that the exponent function is applied to the variable before it is being used in the cumprod function. This is not necessarily a programming bug but more a mathematical error. This also happend to some other variables for which I also made the changes. For most other variables like real_gdp this was already implemented correctly. If you do not apply these changes you can get negative growth numbers which do not make sense. Hope that this makes everything more clear!

@AldoGl
Copy link
Collaborator

AldoGl commented Oct 9, 2024

Thanks @SGHoekstra, now it's clear, well spotted!

I am merging this now, welcome to the contributors of the project! For the moment I will add you here here but we might need to add an "Authors" file at some point. Once again, don't hesitate to push further improvements

@AldoGl AldoGl merged commit 7b24bfe into bancaditalia:main Oct 9, 2024
2 checks passed
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.

2 participants