Closed JakobBD closed 2 months ago
I think it would be generally beneficial to separate out different changes into different pull requests, instead of lumping them together. In this case:
* setting new prtp/ies parameters * removing explicit SE tracking for industry FE * making PBS the default
From the PR title, nobody will guess all this is going on here. Also, all three of these ought to be mentioned in the changelog.
In general, I agree that the best way would be to have only one topic in each PR. But here removing explicit SE tracking for industry FE
and making PBS the default
are to closely related that they should be merged in one PR. In addition, setting new prtp/ies parameters
would also require a recalibration merging it to one PR reduces the number of recalibrations and avoids having REMIND with no correlated CES parameters available.
I agree that Marian's changes should have been a different PR.
removing explicit SE tracking for industry FE
was a different PR #1659
Changelog will still be updated, that's why the box is still un-ticked. Fair point though, it wasn't clear from the PR that this is still planned.
Full PDFs are in cloud folder, link was posted in mattermost REMIND channel.
removing explicit SE tracking for industry FE
was a different PR #1659
Indeed it was. I should look more closely at changed equations.
While I still believe some vetting of AMT CS2 results prior to merge would be better/safer, the PR is now ready for merge, as far as I can see. Calibration files in the public repo are already updated, and the new hash has been added to this PR.
Purpose of this PR
cm_subsec_model = processes
) the default option.Type of change
(Make sure to delete from the Type-of-change list the items not relevant to your PR)
Checklist:
remind2
where it was neededforbiddenColumnNames
in readCheckScenarioConfig.R in case the PR leads to deprecated switchesFAIL 0
in the output ofmake test
)CHANGELOG.md
has been updated correctlyFurther information (optional):
/p/tmp/jakobdu/remind_pbs/output/