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

refactor cost read-in and cost manipulation - moving all cost related codes to one place, improve documentation #1562

Merged
merged 14 commits into from
Feb 19, 2024

Conversation

cchrisgong
Copy link
Contributor

@cchrisgong cchrisgong commented Feb 19, 2024

Purpose of this PR

refactor cost read in and manipulation part of the core/datainput.gms, so all of the code are close together in one place

Type of change

(Make sure to delete from the Type-of-change list the items not relevant to your PR)

  • Bug fix
  • [ X] Refactoring
  • New feature
  • Minor change (default scenarios show only small differences)
  • Fundamental change
  • This change requires a documentation update

Checklist:

  • My code follows the coding etiquette
  • I performed a self-review of my own code
  • I explained my changes within the PR, particularly in hard-to-understand areas
  • I checked that the in-code documentation is up-to-date
  • I adjusted the reporting in remind2 where it was needed
  • I adjusted forbiddenColumnNames in readCheckScenarioConfig.R in case the PR leads to deprecated switches
  • All automated model tests pass (FAIL 0 in the output of make test)
  • The changelog CHANGELOG.md has been updated correctly

Further information (optional):

  • Test runs are here:
  • before (develop /trunk): /p/tmp/chengong/remind_costrefac/remind/output/testOneRegi_develop_2024-02-19_15.34.40
  • after refactoring: /p/tmp/chengong/remind_costrefac/remind/output/testOneRegi_cost_2024-02-19_15.30.19
  • Comparison of results (what changes by this PR?):

@cchrisgong cchrisgong changed the title Cost refac refactor cost read-in and cost manipulation - moving all cost related codes to one place, improve documentation Feb 19, 2024
*RP* rescale the global CSP investment costs in REMIND: Originally we assume a SM3/12h setup, while the cost data from IEA for the short term seems rather based on a SM2/6h setup (with 40% average CF)
*** Accordingly, also decrease long-term costs in REMIND to 0.7 of the current values
fm_dataglob("inco0","csp") = 0.7 * fm_dataglob("inco0","csp");
fm_dataglob("incolearn","csp") = 0.7 * fm_dataglob("incolearn","csp");


*** --------------------------------------------------------------------------------
****** Regionalize investment cost data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make sure there are never more than 3 asterisks behind each other

;

*** initializing energy service capital
pm_esCapCost(tall,all_regi,all_teEs) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does energy service capital still exist in the latest REMIND version, after transport|complex and buildings|putty were deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are sill some
image
@robertpietzcker

@@ -183,6 +211,9 @@ if (c_techAssumptScen eq 3,

display fm_dataglob;

***---------------------------------------------------------------------------
*** Manipulating global or regional technology data - relative value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only about costs, right? so maybe specify "technology cost data"?

Copy link
Contributor

@robertpietzcker robertpietzcker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks ok, but with this amount of code shifted, there could be errors.

I would propose you merge this to prevent merge conflicts, but do additional tests, namely compare all the parameters that might be involved (pm_data, p_inco0, pm_inco0_t, xxx)

I guess the easiest way would be to write them into a file in both the old and the new version, and do a quick winmerge/diff to see if they are exactly the same.
Please do this for both global and regionalized tech costs, and SSP1, SSP2 and SSP5

Then add a link to the folder where the old/new parameter results are saved

It would be painful if one of them changes without us being aware :-)

@cchrisgong cchrisgong merged commit 91ba17b into remindmodel:develop Feb 19, 2024
2 checks passed
@robertpietzcker
Copy link
Contributor

before we forget about this - did you do the comparison of the parameters before/after for the different model configurations?

@cchrisgong
Copy link
Contributor Author

I did a test one regi in test/scenconfig_quick.csv, I believe it is ssp2? And compared endogenous parameters of capital cost. I'm submitting a patch though on some small things.

Sorry for the big change, but I think it shouldn't affect models

@robertpietzcker
Copy link
Contributor

No, I was talking about my comment in my review:
"but do additional tests, namely compare all the parameters that might be involved (pm_data, p_inco0, pm_inco0_t, xxx)

I guess the easiest way would be to write them into a file in both the old and the new version, and do a quick winmerge/diff to see if they are exactly the same.
Please do this for both global and regionalized tech costs, and SSP1, SSP2 and SSP5

Then add a link to the folder where the old/new parameter results are saved"

so not a "are the results the same in a testOneRegi", but comparing the full parameters after datainput. if you do it in a file (simply write into a file with "put"), it will take 1 click on winmerge to check if they are identical; do this for all the setups (global tech costs; regional tech costs; SSP1; SSP2, SSP5) and then we know everything is 100% correct and we never need to worry if something indeed might have changed.

this is much clearer than doing a REMIND run, because the model is not deterministic and so it will be more challenging to decide "are these minimal differences just a slightly different optimum, or is something actually different in the model"

@cchrisgong
Copy link
Contributor Author

I see, thanks, yes, I think I'll do that till Friday if that's okay. I'll submit a short patch PR just for the obvious things. I haven't used put before, I'm also in Linux system. So it will take me a day or two to get there, I hope that's okay

@robertpietzcker
Copy link
Contributor

sure!
on linux, diff will work as well (or better?) than winmerge :-)

and the put utility is quite straight forward, if I remember correctly.

Thanks!

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