-
Notifications
You must be signed in to change notification settings - Fork 971
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
Parser ES updates #7764 #7767
Parser ES updates #7764 #7767
Conversation
ES : Spanish Peninsula I've replaced ENSTO with the ES parser, which has more detail now, so you'll have to check if it's relevant.
please note that battery and hydro have production and consumption => I merge both value. |
ES-CN-HI => ElHierro
{'correctedModes': [], |
ES-CN-LP => LaPalma
|
ES-CN-IG => Gomera
|
ES-CN-TE => Tenerife
|
ES-CN-GC => GranCanaria The hidden link on web site : https://demanda.ree.es/visiona/canarias/gcanariaau/acumulada/2025-1-25
Vapor turbine is oil in the mapping, need to check this point |
ES-CN-FVLZ => LanzaroteFuerteventura
|
ES-IB-MA => Mallorca
|
ES-IB-ME => Menorca
|
ES-IB-IZ => Ibiza
|
ES-IB-FO => Formentera
|
ES-CE => Ceuta
|
ES-ML => Melilla
|
ES->ES-IB-MA
ES-IB-IZ->ES-IB-MA
ES-IB-MA->ES-IB-ME
|
ES-IB-FO->ES-IB-IZ
|
These exchange are not yet configured but implented AD->ES
ES->MA
ES->FR
ES->PT => This one is not yet in Electricity Map
|
@t-couery this looks great so far! I'll start reviewing it tomorrow but to fix the tests you can run |
Thx, I had already tried using --snapshot-update without success, but after investigation I solved the problem bu uninstall snapshottest : |
Ah yeah! We recently switched snapshot library because snapshottest didn't work reliably and had serilization issues. Nice to see it got resolved though! |
parsers/ES.py
Outdated
ses, | ||
logger, | ||
target_datetime, | ||
) | ||
|
||
exchangeList = ExchangeList(logger) | ||
for event in data: | ||
exchanges = {} | ||
net_flow = 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not good, this will mean the value will always be 0 even if it's missing.
I would recommend saving the exports in one exchange mapping and imports in another and then merging them.
something like in this psudo code:
exchanges = []
for exchange_key in EXCHANGE_MAPPING[sorted_zone_keys]["codes"]:
for event in data:
exchange_list = ExchangeList(logger)
for key in event:
exchange_list.append()
exchanges.append(exchange_list)
return ExchangeList.merge_exchanges(exchanges, logger).toList()
(this can probably be made a bit more efficient)
This will ensure it's not defaulting to zero, that we only record the event if both the imports and exports are known, and re-use existing tested logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, it's been modified
'correctedModes': list([ | ||
'hydro', | ||
]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change should not have occured here. Do you know why hydro was negative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran the parser for a full year for ibiza and tenerife and compared the values that were generated when i calculated the emission factors half a year ago. The values are the same except for the differences due to the fuel mappings. E.g. Reporting gas and oil instead of all oil but gas + oil = old gas.
As far as I understand the mappings haven't changed. And even if they had changed some code would be needed to still report old data with the correct mapping.
@@ -291,31 +368,21 @@ def fetch_production( | |||
check_valid_parameters(zone_key, session, target_datetime) | |||
ses = session or Session() | |||
data_mapping = PRODUCTION_PARSE_MAPPING.copy() | |||
|
|||
## Production mapping override for Canary Islands |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this removed?
This was used to adjust the mappings as different zones use different fuels but use the same name in demanda.ree.es.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I only used the wording on the electricity producer's website. I thought the data was credible and the old code obsolete. Do you think we should replace gas with oil?
Exemple of Tenerife :
#7767 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do could you 0 fill the data for gas? Normally we want to avoid it but otherwise it will keep both the oil and gas values in the database and this is simpler than me manually cleaning up the data for all of these zones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tell me, I don't have an opinion on whether to put back the old code or leave mine or set it to 0 here.
I based my opinion on the https://demanda.ree.es/visiona/canarias/tenerifeau/acumulada/2025-01-29 website.
For Tenerife, with my code it's a majority of gas, with the old code it's mainly oil.
The old code gas + cycle combined => old for Canaries + Ibiza
For Menorca Diesel engines => gaz
## Production mapping override for Canary Islands
# NOTE the LNG terminals are not built yet, so power generated by "gas" or "cc" in ES-CN domain is actually using oil.
# Recheck this every 6 months and move to gas key if there has been a change.
# Last checked: 2022-06-27
if zone_key.split("-")[1] == "CN":
data_mapping["gas"] = "oil"
data_mapping["cc"] = "oil"
if zone_key == "ES-IB-ME" or zone_key == "ES-IB-FO":
data_mapping["gas"] = "oil"
if zone_key == "ES-IB-IZ":
data_mapping["die"] = "gas"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we concluded last time that there where no LNG terminals in the Canary Islands so unless that has changed they simply don't have any gas to burn.
Which is why they had a special mapping that attributed it to oil instead.
Not totally sure about the other islands from the top of my head but I think the reasoning was the same there.
Personally as long as we can back it up I'm fine with either option. As long as it's well founded. Best thing would probably be to reach out to REE and ask them to adjust their data mappings but that could take a while even if they decide to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put the old code back in to keep this mapping and modify the test results accordingly
parsers/test/test_ES.py
Outdated
ES.get_url(ZoneKey("ES-IB-MA"), "2025-01-27"), | ||
content=data.read(), | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it could be a good idea to also add a test for DST for the Canary Islands as the strings are a bit different (1A, 1B instead of 2A, 2B). Also because there are no Canary Islands tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, it's added
One detail I wanted to note regarding the fuel mappings: Upon closer inspection I've noticed that the case of Ibiza is a bit more complicated than I initially thought... We can see that there is a bit of oil use... It is about 3% of all energy use so can't be mapped directly to diesel or gas turbines from ree. As according to the aggregated data that I just scraped from ree page neither gas turbines nor diesel has accounted for 3% of production since data is available.
Also: When I looked in the archive, I found a 3 old backup with the following data: Which is also around 3%. So I would guess oil is used as backup or during startup of the diesel or turbine generators. I think therefore that it would make sense to still map it to gas. Another option would be to map all production to unknown and add some comment in the app. The emission factors from gas could be reused even. The drawback would be that we don't know for sure the percentage, as there are only two data points. At least this is what I understand from all this information. I'm sorry if this is very long, I wanted to be as clear as possible. What do you think of this @VIKTORVAV99 ? |
I'd rather not map it to unknown if it can be avoided. If it's just used as an emergency backup I think it's fine to map it as gas as well. Pretty sure that is happening in our other mappings for say ENTSO-E as well. |
Just fyi, but given your level of understanding I know you've already understood. The parser already returns gas (wich include also arround 3% of oil) for the Ibiza zone with the latest updates. So we're in line with what needs to be done in this case. The row data. 'die' is converted to gaz here
|
Hi, is there anything I can do to help close this RP? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few last things regarding some details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some final notes but looks good to me!
I'll merge it tomorrow (Monday) and see if I can add the new zones before creating a new release as well. Otherwise I'll split it later this week.
Thanks a lot for fixing this!
Co-authored-by: Viktor Andersson <[email protected]>
Co-authored-by: Viktor Andersson <[email protected]>
@VIKTORVAV99 I have just updated my .ipynb (Also on Colab) and added the emission factors for Lanzarote and Fuerteventura zones, so check those out when adding the new zones. |
Will do! |
I just wanted to take a moment to sincerely thank you for your help with the Spanish electricity parser. Your knowledge and expertise were invaluable, and I truly appreciate the time and effort you took to guide me. I had no idea that such a dedicated and competent community existed around this topic, and I’m really grateful to have had the opportunity to learn from you. Your insights made a huge difference, and I wouldn’t have made as much progress without your support. Thanks again for you help. |
Issue
This RP is linked to Canary Islands no data since 23.01.2025, closes: #7764
Fixes: GMM-389
Description
https://demanda.ree.es/ data has changed, this PR brings severals changes linked to this change on :
For the mapping of energy sources, I use this mapping. @silimotion says that thermal renewable can also includes biomass =>
I need help to fix tests ES parser.
Preview
Below, I will detail the 12 zones :
Double check
poetry run test_parser "zone_key"
poetry run format
in the top level directory to format my changes.