Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

WIP implementation of standard API functions #19

Merged
merged 3 commits into from
May 23, 2019
Merged

Conversation

corakingdon
Copy link
Collaborator

@corakingdon corakingdon commented May 23, 2019

@davidanthoff Questions:

  • What should happen if the user calls compute_scc for a year not in the model's time index? the index is 2005:10:2595. Currently if they call compute_scc(year=2020) this will error and say they need to provide a valid year. Do we want to implement interpolation so that if they request a 2020 SCC value, it would interpolate between a 2015 SCC value and a 2025 SCC value? this would cause problems for the compute_scc_mm function since more than one marginalmodel is used.
  • Do we want to implement Ramsey discounting option? Currently the only available discounting keyword argument is prtp and constant discounting is used.
  • Since there's currently no ramsey discounting, I made the default prtp value be 0.03. Let me know if I should change this.
  • How exactly do we want to calculate the SCC since marginal damages are on 10 year timesteps? Currently I implemented the simplest way of just discounting each 10yr value then multiplying each value by ten. Alternatively, we could interpolate (or do a step function) to have annual marginal damages, then discount each annual marginal damage value.

And I still need to update the README

@corakingdon corakingdon requested a review from davidanthoff May 23, 2019 22:10
@davidanthoff
Copy link
Member

I think throwing an error for non-existing years is the right way.

I think we should compute the SCC differently: we should simply take the difference between welfare.UTILITY between the two runs. That will give us the SCC in welfare units, so we then still need to normalize with marginal welfare of the period of the emission pulse. Should we merge this PR as is, and then I make the changes to use that method, and you can use it as a template for the other models from that family?

@davidanthoff davidanthoff merged commit c3bf8b3 into master May 23, 2019
@davidanthoff davidanthoff deleted the standard-api branch May 23, 2019 23:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants