electricitymaps-contrib icon indicating copy to clipboard operation
electricitymaps-contrib copied to clipboard

Split Sweden into subzones.

Open VIKTORVAV99 opened this issue 2 years ago • 37 comments

This PR splits Sweden to the subzones SE-SE1, SE-SE2, SE-SE3 and SE-SE4 but should NOT be merge until #1397 is unblocked.

TODO:

  • [x] Add subzone capacities. (Looks like the source includes small scale gasoline and diesel backups in gas but it's the only one I could find that splits it per bidding zone. We should try and find a better source in the future)
  • [x] Update exchanges.
  • [x] Update parsers.
  • [x] Change parsers in zones.json and add subZoneNames to SE.
  • [x] Add gCO2eq/kWh for unknown sources. See #3915 for reference.
  • [x] Update world.geojson with SE-SE1, SE-SE2, SE-SE3, SE-SE4 and remove SE. Help Needed!
  • [x] Update translation files with new names for SE subzones.
  • [x] Update tests.

I know there is already a PR that aims to do the same thing but it has become outdated and need a lot of edits to get in the right shape to be merged, this PR aims to fix that.

PR:s and commits to this PR are welcome!

Fixes: #1397, #2699

VIKTORVAV99 avatar Apr 04 '22 20:04 VIKTORVAV99

Thanks for this PR @VIKTORVAV99 — super great to be able to get this one going finally. We're holding off on this one because we don't yet have the ability to have two regional granularities flowtranced at the same time (ie SE and SE subzones). And we don't want to switch to subzones and get rid of SE until we have this.

For more context, see also similar issues for https://github.com/electricitymap/electricitymap-contrib/issues/3795 (Germany) and https://github.com/electricitymap/electricitymap-contrib/issues/2777 (France)

Once this is implemented (hopefully it will be integrated on the road map this year), we will be in a position to merge this.

But thanks so much for getting the work started on this!

gwpicard avatar Apr 08 '22 08:04 gwpicard

@gwpicard I am aware of the current constraints but my goal with this PR is not to get it merged right away but to have everything ready by the point you can flowtrace multiple granularities. Hopefully, I (we?) can maintain this PR until that point and then have a simple merge without much delay.

VIKTORVAV99 avatar Apr 08 '22 08:04 VIKTORVAV99

@gwpicard I am aware of the current constraints but my goal with this PR is not to get it merged right away but to have everything ready by the point you can flowtrace multiple granularities. Hopefully, I (we?) can maintain this PR until that point and then have a simple merge without much delay.

Yep, agreed. I don't think we should close it but just wanted to flag it so we're aligned.

However, we should probably close https://github.com/electricitymap/electricitymap-contrib/issues/1397

gwpicard avatar Apr 08 '22 08:04 gwpicard

@gwpicard, Issue #1397 has become outdated and should be closed but it might make sense to open a new issue, partially to prevent someone else from creating a duplicate and to have a place to leave comments not related to the code/changes in this PR.

VIKTORVAV99 avatar Apr 08 '22 08:04 VIKTORVAV99

@gwpicard, Issue #1397 has become outdated and should be closed but it might make sense to open a new issue, partially to prevent someone else from creating a duplicate and to have a place to leave comments not related to the code/changes in this PR.

I think we should keep #1397 because it has useful context and past conversations relevant to this PR. But yes, now that it's tagged again here, people can add comments over on the issue rather than on this PR.

gwpicard avatar Apr 08 '22 09:04 gwpicard

cc @pierresegonne for visibility

corradio avatar Apr 12 '22 17:04 corradio

@pierresegonne While maintaining this PR I noticed that isort is passing locally but not in the CI, any reason for that?

VIKTORVAV99 avatar Jun 07 '22 14:06 VIKTORVAV99

@pierresegonne While maintaining this PR I noticed that isort is passing locally but not in the CI, any reason for that?

I guess it might be a version issue? What version do you have locally?

pierresegonne avatar Jun 09 '22 16:06 pierresegonne

@pierresegonne While maintaining this PR I noticed that isort is passing locally but not in the CI, any reason for that?

I guess it might be a version issue? What version do you have locally?

My local version is 5.9.3, the same version that is specified in pyproject.toml

VIKTORVAV99 avatar Jun 10 '22 16:06 VIKTORVAV99

Hey @VIKTORVAV99 :)

When pulling your changes locally and running poetry run check isort fails for me. So I guess it's because your isort config is outdated or something along these lines. You could remove and reinstall without cache to double check.

In the meanwhile, these are the imports in test_parser.py that should make isort happy:

import datetime
import logging
import pprint
import time

import arrow
import click

from electricitymap.contrib.parsers.lib.parsers import PARSER_KEY_TO_DICT
from parsers.lib.quality import (
    ValidationError,
    validate_consumption,
    validate_exchange,
    validate_production,
)

pierresegonne avatar Jun 15 '22 13:06 pierresegonne

Hey @VIKTORVAV99 :)

When pulling your changes locally and running poetry run check isort fails for me. So I guess it's because your isort config is outdated or something along these lines. You could remove and reinstall without cache to double check.

In the meanwhile, these are the imports in test_parser.py that should make isort happy:

import datetime
import logging
import pprint
import time

import arrow
import click

from electricitymap.contrib.parsers.lib.parsers import PARSER_KEY_TO_DICT
from parsers.lib.quality import (
    ValidationError,
    validate_consumption,
    validate_exchange,
    validate_production,
)

Thanks I'll give it a try.

On a slightly off topic note is there a reason we are importing from electricitymap.contrib.parsers.lib.parsers instead of just parsers.lib.parsers? (like the one just after)

VIKTORVAV99 avatar Jun 15 '22 15:06 VIKTORVAV99

Hey @VIKTORVAV99 :) When pulling your changes locally and running poetry run check isort fails for me. So I guess it's because your isort config is outdated or something along these lines. You could remove and reinstall without cache to double check. In the meanwhile, these are the imports in test_parser.py that should make isort happy:

import datetime
import logging
import pprint
import time

import arrow
import click

from electricitymap.contrib.parsers.lib.parsers import PARSER_KEY_TO_DICT
from parsers.lib.quality import (
    ValidationError,
    validate_consumption,
    validate_exchange,
    validate_production,
)

Thanks I'll give it a try.

On a slightly off topic note is there a reason we are importing from electricitymap.contrib.parsers.lib.parsers instead of just parsers.lib.parsers? (like the one just after)

@pierresegonne turns out when using the import electricitymap.contrib.parsers.lib.parsers isort identifies it as a THRIDPARTY module and therefor groups it with the other third party modules but when using the shorter parsers.lib.parsers import isort identifies it as a FIRSTPARTY module just as parsers.lib.quality.

I don't know if this is affected by the operating system (I think it is since windows don't appears to be able to resolve the longer symbolic import) but I think we should switch to the shorter import just to be safe. I'll open a separate PR for that and we can discuss it more there if there is any issues with going down that path.

VIKTORVAV99 avatar Jun 15 '22 20:06 VIKTORVAV99

On a slightly off topic note is there a reason we are importing from electricitymap.contrib.parsers.lib.parsers instead of just parsers.lib.parsers? (like the one just after)

From what I know, the first is the poetry style import, and the second is vanilla python.

I wouldn't be able to tell you why one would prefer one rather than the other, so feel free to open a different PR with that, and ask Mads' input :)

pierresegonne avatar Jun 16 '22 06:06 pierresegonne

Hey @VIKTORVAV99, the situation is unlocked on our side, so we can start looking into merging this finally :)

pierresegonne avatar Jul 25 '22 08:07 pierresegonne

Hey @VIKTORVAV99, the situation is unlocked on our side, so we can start looking into merging this finally :)

Awesome! 🎉

It should only be the geographies that's missing, perhaps you could help with those @Kongkille?

(I can't fit the GiS editing software on my hard drive at the moment... 😅)

VIKTORVAV99 avatar Jul 25 '22 09:07 VIKTORVAV99

FYI in #3607 there were some work done on the geometries

pierresegonne avatar Jul 25 '22 14:07 pierresegonne

@VIKTORVAV99 I'll try to add the geographies to this PR!

Kongkille avatar Jul 25 '22 14:07 Kongkille

FYI in #3607 there were some work done on the geometries

It where but it used the old geometry system so we sadly can't just copy the changes in that pr. 😕

VIKTORVAV99 avatar Jul 25 '22 14:07 VIKTORVAV99

Exciting to see this merged and really interesting to read the discussions about the journey! It seems so absurd to me that the Swedish government decided on splitting the grid in 4 zones without opening all the data related to these areas.

Regarding the GIS data, I can try to have a look if no one's working on it. Just let me know.

PierreMesure avatar Jul 25 '22 21:07 PierreMesure

Exciting to see this merged and really interesting to read the discussions about the journey! It seems so absurd to me that the Swedish government decided on splitting the grid in 4 zones without opening all the data related to these areas.

Regarding the GIS data, I can try to have a lot if no one's working on it. Just let me know.

The data has been available for a while on ENTSO-E and has been released as yearly reports. It has just been some blocking factors that are now resolved.

As for the geographies @Kongkille is working on those but you can always double check with him.

Otherwise I don't think he'd mind if you helped out with the geographies for Ceuta and Melilla in #4318. They are very small zones but it would be nice to get them added all the same.

VIKTORVAV99 avatar Jul 26 '22 05:07 VIKTORVAV99

Geographies seem to be working now :)

image

It was a bit difficult because our validation on checking gaps has a tendency to produce some odd results, so I've included some code in the geo validator to ensure we don't get stuck on small polygon slivers

image

Kongkille avatar Jul 26 '22 09:07 Kongkille

Geographies seem to be working now :)

image

It was a bit difficult because our validation on checking gaps has a tendency to produce some odd results, so I've included some code in the geo validator to ensure we don't get stuck on small polygon slivers

image

Nice!

I think the actual bidding zone borders are a lot weirder (not straight lines) but this should be more than sufficient enough us to deploy it to the map at least. (we can always improve them later if needed, for example I'd love to add the two great lakes at one point)

VIKTORVAV99 avatar Jul 26 '22 10:07 VIKTORVAV99

I'll update the Jest tests to match the new zones config but I think this is ready to merge after that.

VIKTORVAV99 avatar Jul 26 '22 10:07 VIKTORVAV99

@Kongkille I updated the Jest test but it's still failing, is it because there is no exchange data between DK-DK2 and SE-SE4 in the mock data?

VIKTORVAV99 avatar Jul 26 '22 11:07 VIKTORVAV99

@Kongkille I updated the Jest test but it's still failing, is it because there is no exchange data between DK-DK2 and SE-SE4 in the mock data?

Ah yeah I'll just move the Jest test to another zone. We should actually be able to have both SE and SE-SE* but I don't think we need to address this in the dataReducer test.

Kongkille avatar Jul 26 '22 11:07 Kongkille

@VIKTORVAV99

I saw that on SvK the official names for the bidding areas are:

  • Luleå (SE1)
  • Sundsvall (SE2)
  • Stockholm (SE3)
  • Malmö (SE4)

Source

Up to you whether you think we should keep the current names you had, thought it might be nice to align with the official names.

gwpicard avatar Aug 01 '22 12:08 gwpicard

@VIKTORVAV99

I saw that on SvK the official names for the bidding areas are:

  • Luleå (SE1)
  • Sundsvall (SE2)
  • Stockholm (SE3)
  • Malmö (SE4)

Source

Up to you whether you think we should keep the current names you had, thought it might be nice to align with the official names.

It might be the "official" names but I used the names that E.on (a major electricity utility company) used as they are more descriptive and they translate way better.

VIKTORVAV99 avatar Aug 01 '22 12:08 VIKTORVAV99

In Sweden, these zones are simply called SE1, SE2, SE3, SE4. I can ask my friend at SvK but I've never seen anyone using the city names. I also find it misleading as these zones are much bigger and include other big cities than these 4 (Göteborg, for instance).

PierreMesure avatar Aug 01 '22 12:08 PierreMesure

In Sweden, these zones are simply called SE1, SE2, SE3, SE4. I can ask my friend at SvK but I've never seen anyone using the city names. I also find it misleading as these zones are much bigger and include other big cities than these 4 (Göteborg, for instance).

I think it's just better to go with the more descriptive name that we can translate.

Maybe you can convince your friend at SvK to change the ones SvK use to the ones E.on use. 😉

VIKTORVAV99 avatar Aug 01 '22 12:08 VIKTORVAV99

I actually checked some news articles and many also use the more descriptive names that you (and E.on) use.

PierreMesure avatar Aug 01 '22 13:08 PierreMesure