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

Residential and commercial floor space. #352

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

robinhasse
Copy link
Contributor

@robinhasse robinhasse commented Jan 25, 2023

Instead of filtering the total floor space, we now write residential, commercial and total floor space into p36_floorspace.cs4r. This allows to report subsector floor space in remind2. See #379 for those changes.

@codecov
Copy link

codecov bot commented Jan 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (31573be) 0.00% compared to head (c22a9b7) 0.00%.

❗ Current head c22a9b7 differs from pull request most recent head 69b361c. Consider uploading reports for the commit 69b361c to get more accurate results

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #352   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         265     265           
  Lines       17266   17271    +5     
======================================
  Hits            1       1           
- Misses      17265   17270    +5     
Files Changed Coverage Δ
R/calcFloorspace.R 0.00% <0.00%> (ø)
R/fullREMIND.R 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

calcFloorspace <- function() {

data <- readSource("EDGE", subtype = "Floorspace")
data <- collapseNames(data[,,"buildings"])
Copy link
Member

Choose a reason for hiding this comment

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

Barging in here without being asked: this has the (undesired) side effect of cutting all REMIND versions without remindmodel/remind#1197 off from new input data.
All project branches (SDP, Tokyo stuff, I don't know) that for whatever reason don't merge the REMIND changes will not be able to produce new input data, even if they need some because they did change something on their end.
I don't know which project needs new input data, and the REMIND changes are quite isolated and can easily be cherry-picked (I think), but it would be to put this behind a subtype parameter to allow them to toggle this.
If fullREMIND() would produce two files from this data, one with and one without the collapsed dimension, projects could go on using input data without intervention.

Copy link
Contributor Author

@robinhasse robinhasse Jan 26, 2023

Choose a reason for hiding this comment

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

I see the point and adding this is no effort. On the other hand, using newer input data will be impossible in many occasions. We just broke this option with the introduction of a new scenario that is now part of the input data and will give you a Domain violation Error with any REMIND version that is more than one week old.

Choose a reason for hiding this comment

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

Ahh, wasn't aware of that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So after some discussion some time ago also with @LaviniaBaumstark, I decided to create a new file f36_floorspace.cs4r with the sub-sectoral floor space and keep the old p36_floorspace.cs4r for a while. Once input data eith the new file is available, I will adapt REMIND and remind 2 to make use of it. This PR can be merged with no side-effects and compatibility issues expected.

Copy link
Member

@LaviniaBaumstark LaviniaBaumstark left a comment

Choose a reason for hiding this comment

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

merge needs to be coordinated with changes in REMIND

@robinhasse
Copy link
Contributor Author

merge needs to be coordinated with changes in REMIND

As I now leave the old file p36_floorspace.cs4r unchanged, there is no coordination required anymore. Would be great to merge it soon to have the new file in the next input data.

@robinhasse robinhasse merged commit 7e4728a into pik-piam:master Aug 21, 2023
1 check passed
@robinhasse robinhasse deleted the resComFloorReporting branch August 21, 2023 07:10
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.

3 participants