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.

Sign up for GitHub

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

Jump to bottom

Split Italy #1604

Merged
merged 11 commits into from
Sep 20, 2018
Merged

Split Italy #1604

merged 11 commits into from
Sep 20, 2018

Conversation

corradio
Copy link
Member

@corradio corradio commented Sep 17, 2018 •
edited
Loading

Ref #1029
Superseeds #1536

Needs to be done:

  • Add geometries
  • Add ENTSOE exchange mappings
  • Add ENTSOE production mappings
  • Add bounding boxes

@corradio corradio mentioned this pull request Sep 17, 2018
Closed
@corradio
Copy link
Member Author

This is what it looks like for now:

untitled9

@alixunderplatz
Copy link
Collaborator

@corradio Looks very good so far!

@alixunderplatz alixunderplatz changed the title [WIP] Split Itality [WIP] Split Italy Sep 17, 2018
@corradio
Copy link
Member Author

corradio commented Sep 18, 2018 •
edited
Loading

@alixunderplatz what do you think about both SACOAC/SACODC <-> IT-SAR: should we sum them?
Second question: it's quite annoying to add several bidding zones together. Do you think we can just ignore the smaller ones or would that be wrong?

@brunolajoie
Copy link
Contributor

brunolajoie commented Sep 18, 2018 •
edited
Loading

Can't wait to see it live. We should also investigate what is the yearly average "unknown" fuel mix in each of these region to refine carbon intensities!

@alixunderplatz
Copy link
Collaborator

@corradio regarding your second question: unfortunately, we'd have to summarize the bidding zones, because some of the micro zones have significant generation (like +2GW in Brindisi and Rossano).
Brindisi, Foggia and Priolo could be displayed as seperate provinces if we take their surrounding administrative regions. The province around Rossano (Consenza) would sort of "block" the part towards Sicily from the main part of the Italian boot, and below you'd have another part of Southern Italy. Personally, I would not want to "spoil" these areas with the micro-zones' CO2-intensity, and I think this would only confuse users, that's why I suggest including them in their larger surrounding area. I'm sorry implementing this is difficult :/

@alixunderplatz
Copy link
Collaborator

@corradio regarding your first question: Maybe we get the everything on the mainland right, and then continue with Sardinia in another issue/PR. The grid infrastructure is a bit complicated and I can't quickly answer your question before the weekend.
A part of the energy in the DC-line between IT-CNO and Sardinia could be leaving the line in Corsica, so the amount reaching Sardinia may be a bit smaller. See #1029
I am not sure how to draw the exchange arrows, too.
If somebody could check all the exchanges on ENTSO-E related to Sardinia / Corsica, this would be terrific.

@alixunderplatz
Copy link
Collaborator

Just a quick summary regarding Sardinia for what you see in "Cross-Border physical flow" on ENTSO-E selecting "border-bidding zone":

"IT-Centre-North-IT-SACODC" is the exchange between IT-CNO and the DC-cable to Sardinia/Corsica.
"IT-SACODC-IT-Sardinia" is the exchange with the end of the DC-cable and Sardinia (the real exchange IT-CNO <-> Sardinia).
The difference is leaving the line on Corsica (IT-CNO <-> Corsica).
"IT-SACOAC-IT-Sardinia" is an AC-exchange Sardinia <-> Corsica.
Not sure how often the case appears that energy is fed from Sardinia towards IT-CNO and a part is leaving in Corsica. This is what "worries" me the most, although the max. power ever leaving in Corsica should be ~50 MW.

Carbon intensity on Corsica will remain unknown due to a lack of generation data.

Maybe handling SACODC as pseudo-zone for the map will make things a bit easier? Sorry if this complicates matters even more :/

@corradio
Copy link
Member Author

corradio commented Sep 19, 2018 •
edited
Loading

@alixunderplatz I've left out the SACODC/AC for now.
Please review very carefully and check I've added all imports/exports and properly summed up the zones.

image

@corradio corradio changed the title [WIP] Split Italy Split Italy Sep 19, 2018
maxbellec
maxbellec reviewed Sep 19, 2018
View reviewed changes
.circleci/config.yml
@@ -34,7 +34,7 @@ stages:
flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics
# exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
pylint -E parsers/*.py
pylint -E parsers/*.py -d unsubscriptable-object,unsupported-assignment-operation
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because of this: pylint-dev/pylint#1498

parsers/ENTSOE.py Outdated Show resolved Hide resolved
@maxbellec
Copy link
Contributor

I found _sum_production hard to understand so I rewrote it. If you prefer the original version, we can roll back.

@corradio
Copy link
Member Author

@maxbellec did you test that it returns exactly the same results? It's counter-productive if I now have to review your changes

@maxbellec
Copy link
Contributor

yes the ouput is exactly the same for fetch_production('IT-SIC')

@corradio
Copy link
Member Author

corradio commented Sep 19, 2018 via email

@maxbellec
Copy link
Contributor

Sorry, I know generally it's better to comment than change directly but I thought the entire function had to be re-written if we wanted something clean.
Just to check: when merging productions from zone A and B, we only keep datapoints whose datetime are in A production and B production, right? Otherwise we'll end up with wrong data I believe.

maxbellec
maxbellec reviewed Sep 19, 2018
View reviewed changes
web/generate-zone-bounding-boxes.js
function readNDJSON(path) {
return fs.readFileSync(path, 'utf8').split('\n')
.filter(d => d !== '')
.map(JSON.parse);
}

let zones = readNDJSON('build/zonegeometries.json');
if (args.length > 0) {
zones = zones.filter(d => d.properties.zoneName === args[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@maxbellec
Copy link
Contributor

haven't checked the mappings yet though

@corradio
Copy link
Member Author

yes that's right.

@alixunderplatz
Copy link
Collaborator

@corradio micro-zones are summed correctly with the larger zones. All exchanges are included
👍 👍 👍

@corradio
Copy link
Member Author

@alixunderplatz great! Can you approve the PR then?

brunolajoie
brunolajoie approved these changes Sep 20, 2018
View reviewed changes
Copy link
Contributor

@brunolajoie brunolajoie left a comment

Choose a reason for hiding this comment

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

👏

@corradio corradio merged commit 27d9f7e into master Sep 20, 2018
@corradio corradio deleted the olc/split-italy branch September 20, 2018 14:33
@lorrieq lorrieq mentioned this pull request Oct 26, 2018
Merged
@alixunderplatz alixunderplatz mentioned this pull request Oct 16, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brunolajoie brunolajoie brunolajoie approved these changes

@maxbellec maxbellec maxbellec approved these changes

@alixunderplatz alixunderplatz Awaiting requested review from alixunderplatz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@corradio @alixunderplatz @brunolajoie @maxbellec