Apply shift in investment variables #664
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orichters
reviewed
Oct 21, 2024
There was a problem hiding this comment.
Thanks for implementing that. I have some small comments scrolling through the PR, but have no time to look deeper. Do you have some comparison plots of old vs. new for these variables? I think that would be helpful to see whether the smoothing works and the variables still have the same order of magnitude. Regarding the fixing, I have lots of trust into piamInterfaces::fixOnRef :)
R/reportCapacity.R
Outdated
| @@ -10,6 +10,8 @@ | |||
| #' be created. | |||
| #' @param t temporal resolution of the reporting, default: | |||
| #' t=c(seq(2005,2060,5),seq(2070,2110,10),2130,2150) | |||
| #' @param gdx_ref a GDX object as created by readGDX, or the path to a gdx of the reference run. | |||
| #' It is used to guarantee consistency for Moving Avg prices before cm_startyear | |||
There was a problem hiding this comment.
Suggested change
| #' It is used to guarantee consistency for Moving Avg prices before cm_startyear | |
| #' It is used to guarantee consistency before cm_startyear for Capacity variables where some time-averaging is used |
There was a problem hiding this comment.
Has been adjusted
R/reportLCOE.R
Outdated
| @@ -20,6 +20,8 @@ | |||
| #' Can be either "average" (returns only average LCOE), | |||
| #' "marginal" (returns only marginal LCOE), "both" (returns marginal and average LCOE) and | |||
| #' and "marginal detail" (returns table to trace back how marginal LCOE are calculated). | |||
| #' @param gdx_ref a GDX object as created by readGDX, or the path to a gdx of the reference run. | |||
| #' It is used to guarantee consistency for Moving Avg prices before cm_startyear | |||
There was a problem hiding this comment.
Really prices here, or is that copied from reportPrices?
There was a problem hiding this comment.
Has been adjusted
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a follow-up to remindmodel/remind#1238
It applies a time shift to the following investment-related variables:
vm_deltaCap,vm_costInvTeDirandvm_costInvTeAdj, affecting the variablesNew Cap|...,Cumulative Cap|...,Energy Investments|...andTotal Energy costs|...Validation:
piamInterfaces::fixOnRefdid not yield any new isues