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

update(parser) - ELEXON #6441

Merged
merged 16 commits into from
Jun 3, 2024
Merged

update(parser) - ELEXON #6441

merged 16 commits into from
Jun 3, 2024

Conversation

ghost
Copy link

@ghost ghost commented Feb 2, 2024

Issue

Description

Here is a diagram of how the data is collected
gb parser

We assume that the hydro storage from ESO is only pumping (from the grid to the reservoir) because it happens in the night and B1620 is discharge (from the reservoir to the grid) because it happens during the day at what see to be peak hours
image

This means that storage from B1620 is counted as negative and storage from ESO is counted as >0

Preview

Double check

  • I have tested my parser changes locally with poetry run test_parser "zone_key"
  • I have run pnpx prettier@2 --write . and poetry run format in the top level directory to format my changes.

@github-actions github-actions bot added parser python Pull requests that update Python code labels Feb 2, 2024
@VIKTORVAV99
Copy link
Member

VIKTORVAV99 commented Feb 7, 2024

@ghost
Copy link
Author

ghost commented Feb 7, 2024

Have you seen this new Elexon API? developer.data.elexon.co.uk/api-details#api=prod-insol-insights-api&operation=get-datasets-fuelinst

I have not looked through them all yet but there could be some nice gems in there.

yes actually the revamp of the parser uses the new API. It's just that i still need to look at the pumping data before the PR is ready for review

@ghost ghost marked this pull request as ready for review February 20, 2024 14:41
@ghost ghost requested a review from VIKTORVAV99 as a code owner February 20, 2024 14:41
@ghost ghost changed the title elexon parser update(parser) - ELEXON Feb 20, 2024
Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

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

Lots of small notes that needs to be addressed.

I'm also seeing a lot of these in the terminal:

WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`
WARNING:test_parser:GB reported total of 0.00MW falls outside range of (100, 60000) for key `gas`

Is this expected? I don't see why they would report 0 gas (and nuclear).

parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Outdated Show resolved Hide resolved
storage_breakdown = ProductionBreakdownList(logger=logger)
for event in eso_data:
event_datetime = parse_datetime(
event.get("SETTLEMENT_DATE"), event.get("SETTLEMENT_PERIOD")
Copy link
Member

Choose a reason for hiding this comment

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

While it never should be none we need to make handle it to satisify the typing (with isinstance for example).

parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Show resolved Hide resolved
parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Feb 21, 2024

will remove this parser validation. I don't think it makes a lot of sese

@ghost ghost requested a review from VIKTORVAV99 February 21, 2024 12:17
@VIKTORVAV99
Copy link
Member

I'll look into this further but there seems to be something seriously wrong with the earlier data:

{'correctedModes': [],
  'datetime': datetime.datetime(2024, 2, 19, 0, 0, tzinfo=datetime.timezone.utc),
  'production': {'biomass': 0.0,
                 'coal': 0.0,
                 'gas': 0.0,
                 'hydro': 0.0,
                 'nuclear': 0.0,
                 'oil': 0.0,
                 'solar': 0.0,
                 'unknown': 0.0,
                 'wind': 10108.67},
  'source': 'elexon.co.uk, nationalgrideso.com',
  'sourceType': <EventSourceType.measured: 'measured'>,
  'storage': {'hydro': 11.0},
  'zoneKey': 'GB'}

(This is after I changed so 0 is not considered false so we get 0 solar at night).
I think this stems from the fact that we use two different endpoints? And therefore there is a misalignment?

Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

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

If I go to https://data.elexon.co.uk/bmrs/api/v1/datasets/FUELHH/stream?publishDateTimeFrom=2024-02-10&publishDateTimeTo=2024-02-11 everything seems to be fine data wise.

But if I run poetry run test_parser GB --target_datetime="2024-02-11" I get a bunch of zero filled data which indicated there is something flawed in either the datasets or our logic.

Turns out it's probably both. I have adressed them in individual comments.

parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Outdated Show resolved Hide resolved
Comment on lines 427 to 429
data = ProductionBreakdownList.merge_production_breakdowns(
[data, parsed_hydro_storage_data], logger, matching_timestamps_only=True
).to_list()
Copy link
Member

Choose a reason for hiding this comment

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

Here we are once again merging these datapoints that was merged in the query_and_merge_production_fuelhh_and_eso function, effectivly merging them twice.

Copy link
Author

Choose a reason for hiding this comment

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

no that is not what is happening here. We are only collecting hydro storage data from the eso source and merging it with the B1620 data that only has discharge so it is not double counting

parsers/ELEXON.py Show resolved Hide resolved
@q--
Copy link
Contributor

q-- commented Feb 21, 2024

Adding a link to #6419 so its relationship to this pull request is clear in GitHub (though I'm unclear on whether this pull request addresses the issue, or that it was merely looked into).

@ghost ghost marked this pull request as draft February 22, 2024 10:04
@ghost
Copy link
Author

ghost commented Feb 22, 2024

converting back to draft, i will address this next week.

@ghost
Copy link
Author

ghost commented Mar 27, 2024

Adding a link to #6419 so its relationship to this pull request is clear in GitHub (though I'm unclear on whether this pull request addresses the issue, or that it was merely looked into).

thanks for the link. This is indeed an answer to the issue you raised about discharging at night :)

From my investigation, it seems that ESO data only covers pumping and not discharge and that is why the data is off, it is missing the other half of the puzzle!

@ghost ghost marked this pull request as ready for review March 27, 2024 14:49
@ghost
Copy link
Author

ghost commented Mar 27, 2024

image

@ghost ghost requested a review from VIKTORVAV99 March 27, 2024 15:18
Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

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

Seems to be working but there is still the issue of no data at night (it's missing when it should be 0).

parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Outdated Show resolved Hide resolved
parsers/ELEXON.py Show resolved Hide resolved
report["expected_fields"], len(df.columns)
exchange_data = query_elexon(
ELEXON_URLS["exchange"], session, exchange_params
).get("data")
Copy link
Member

Choose a reason for hiding this comment

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

There is no guarantee that get is available since the response is a JSON response, we need to ensure it is a dict before calling get on it.

The type data also says it will return a list, which causes type issues.

Copy link
Member

Choose a reason for hiding this comment

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

This is just a type issue so ignoring it for now since we need to get this merged ASAP.

@VIKTORVAV99 VIKTORVAV99 self-assigned this May 22, 2024
Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

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

I did some small changes and now the output looks a bit better with less errors by correcting negative values with 0 (so they are still valid) and by only merging the matching timestamps (so we ensure we get complete data before the validation method kicks in).

There might still be an issue with no solar at all during the night (not null or 0, simply missing) but it's not blocking from my side. So I'll leave that up to you.

I'm going to bussy for the rest of this and next week so I won't be able to fix that particular issue if you consider it blocking.

@VIKTORVAV99
Copy link
Member

@tonypls @madsnedergaard could you just give this a quick double check?
I know there is a lot of changes and they should all be fine but it would be nice to get another set of eyes on it. We need to merge this before the 1st so it's kind of urgent as they will be shutting down the old API.

So I'll be merging this tomorrow no matter what so we can monitor it during Friday if needed.

Copy link
Member

@madsnedergaard madsnedergaard left a comment

Choose a reason for hiding this comment

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

LGTM, thanks both of you for the work here ❤️

@VIKTORVAV99 VIKTORVAV99 enabled auto-merge (squash) May 29, 2024 14:15
@VIKTORVAV99 VIKTORVAV99 merged commit 51c3210 into master Jun 3, 2024
19 checks passed
@VIKTORVAV99 VIKTORVAV99 deleted the md/update-elexon-parser branch June 3, 2024 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants