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

Minor fixes for retrofit calculations #767

Merged
merged 7 commits into from
Oct 26, 2023
Merged

Conversation

ekatef
Copy link
Contributor

@ekatef ekatef commented Oct 25, 2023

Changes proposed in this Pull Request

Fixing some minor discrepancies for buildings area calculations in build_retro_cost script. In particular:

  1. U-values for Poland are represented in hotmaps dataset as Construction features (U-value) instead of Construction features (U-values);
  2. "Total" feature is contained in subsector column, not in detail one.

Checklist

  • I tested my contribution locally and it seems to work fine.
  • Code and workflow changes are sufficiently documented.
  • Changed dependencies are added to envs/environment.yaml.
  • Changes in configuration options are added in all of config.default.yaml.
  • Changes in configuration options are also documented in doc/configtables/*.csv.
  • A release note doc/release_notes.rst is added.

@martacki
Copy link
Member

martacki commented Oct 25, 2023

The solar global radiation was somehow off, our updated values are taken from this source (which is also referenced in the script): http://www.episcope.eu/fileadmin/tabula/public/docs/report/TABULA_CommonCalculationMethod.pdf

Copy link
Member

@martacki martacki left a comment

Choose a reason for hiding this comment

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

With the suggested changes, I was able to reproduce the retrofitting costs from "Mitigating Heat Demand Peaks" Study, Supplementary Material, Table 3. 🎉

Just a quick note, in line 950, I need to un-comment renaming the column from country to country_code, otherwise I'll get an error (country_code does not exist). But could be that it's just me?

@fneum
Copy link
Member

fneum commented Oct 25, 2023

Could you confirm whether a renaming of country/country code is necessary?

Then, this can be merged.

@yerbol-akhmetov
Copy link
Contributor

Good day, I have also applied the changes and observed the produced retrofitting costs. The resulting file is quite similar to the aforementioned paper's results in terms of order of scale. I have observed some differences for various countries (AT, BA and etc). Regarding uncommenting line 950, I have first tried as it is (commented) and I did not see any error. When I have uncommented it, I have faced an error.

@martacki
Copy link
Member

@ekatef and I have discussed the above mentioned issue about renaming, but it turns out that the error is on my end. Not sure what's wrong, but I think it's clean as is. Ready to merge.

@fneum fneum merged commit f35ecbe into PyPSA:master Oct 26, 2023
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.

4 participants