Techspecific drate

[FraGard]

I changed the parameter DiscountRate[r] into DiscountRate[r,t] in all three codes and relative examples. I also added back parameter DiscountRateStorage[r,s] (for compatibility reasons).

I made mistakes in updating the fast and short code, hence the high number of commits in the past two days.

I noticed than in the meantime the DiscountRate has been taken off the master code and replaced by the CapitalRecoveryFactor, so I guess these codes are incompatible with the Master.

[tniet]

Hi @FraGard - yes, there is a conflict between the CapitalRecoveryFactor and the technology specific discount rate. I think that both pieces are valuable contributions to making the code better so now it's a process of merging the conflicts to create a version without conflicts. Hopefully that's not to challenging since you've done all the work with the technology specific discount rate already?

[willu47]

Hi @tniet and @FraGard. This is partially my fault, but it's also what git is good at managing. What has happened is that I have made some changes to the same bit of code that you are changing.

I also used bad practice - I didn't create an issue to flag what I was changing. I modified the master branch directly, and as such I didn't create a pull request. Hence there's no record of the reasoning behind the changes other than the commits, which are 0cbffb966ca5b15707a4347e00851b2bcaeaa565 through to e982aed539564a9df04baf01950f90f08a9b86a8.

I'll describe what I was trying to do here, but use this as a cautionary tale for why we have a (well documented) procedure to follow and what happens if someone (and I should know better) tries to avoid them!

In the commits above, I try to avoid the duplication across the formulation for each of the uses of discount rate. What we essentially do is calculate a capital recovery factor, or equivalent, each time we use discount rate. My changes replace these duplicate calculations with calculated parameters CapitalRecoveryFactor and CapitalRecoveryFactorMid.

param CapitalRecoveryFactor{r in REGION, y in YEAR} :=
    (1 + DiscountRate[r]) ^ (y - min{yy in YEAR} min(yy) + 0.0);
param CapitalRecoveryFactorMid{r in REGION, y in YEAR} :=
    (1 + DiscountRate[r]) ^ (y - min{yy in YEAR} min(yy) + 0.5);

And that's why you see so many conflicts, because the references to DiscountRate have been replaced with one of the new parameters.

The good news is that having removed the duplicate code, there's fewer places you need to update the DiscountRate parameter with its technology specific version. The bad news is that it may be quicker to start again, rather than go through the hassle of trying to fix all the conflicts.

[tniet]

Hi @FraGard - I think the result of the review is that we can't merge this into the master unless it's adjusted to match the new structure that @willu47 implemented and the conflicts are resolved. Sorry that this is asking you to do additional work to get this merged into the master branch.

[FraGard]

Dear @tniet, @willu47, thanks! Indeed I had identified the conflicts. For me it was more a matter of whether, in terms of procedure, it is ok for me to solve the conflicts online, on the editor of GitHub, or it should be the admin. In any case, thanks @willu47 for fixing it.

On the idea of having all versions in one, not sure. I think the short one could be archived, like suggested by @abhishek0208. And the long one would probably have to be a very straightforward code for beginner / non-coders.