43 new variable statistics final energy by carrierelectricity#47
Merged
maxnutz merged 9 commits intoApr 25, 2026
Merged
Conversation
7 tasks
Contributor
Reviewer's GuideRefactors the Final_Energy_by_Carrier__Electricity statistics function to use sector-specific withdrawal queries aligned with pypsa-de logic, and adds focused tests using a minimal mock statistics accessor to validate filtering, ordering, and query patterns. Sequence diagram for refactored Final_Energy_by_Carrier__Electricity statistics functionsequenceDiagram
participant Caller
participant FinalEnergyByCarrierElectricity
participant Network_n
participant StatisticsAccessor as n_statistics
Caller->>FinalEnergyByCarrierElectricity: call(n, aggregate_per_year)
activate FinalEnergyByCarrierElectricity
Note over FinalEnergyByCarrierElectricity: Final Energy|Agriculture|Electricity
FinalEnergyByCarrierElectricity->>Network_n: get statistics
Network_n-->>FinalEnergyByCarrierElectricity: n.statistics
FinalEnergyByCarrierElectricity->>n_statistics: withdrawal(carrier=[agriculture electricity, agriculture machinery electric], components=Load, aggregate_time=aggregate_per_year, kwargs)
n_statistics-->>FinalEnergyByCarrierElectricity: agri Series
Note over FinalEnergyByCarrierElectricity: Final Energy|Residential and Commercial|Electricity
FinalEnergyByCarrierElectricity->>n_statistics: withdrawal(bus_carrier=low voltage, aggregate_time=aggregate_per_year, kwargs_filtering)
n_statistics-->>FinalEnergyByCarrierElectricity: lv Series
FinalEnergyByCarrierElectricity->>FinalEnergyByCarrierElectricity: build forbitten_list from lv carrier index using regex patterns
FinalEnergyByCarrierElectricity->>FinalEnergyByCarrierElectricity: rescom = lv excluding forbitten carriers
FinalEnergyByCarrierElectricity->>FinalEnergyByCarrierElectricity: rescom = rescom.groupby(groupby).sum()
Note over FinalEnergyByCarrierElectricity: Final Energy|Transportation|Electricity
FinalEnergyByCarrierElectricity->>n_statistics: withdrawal(bus_carrier=low voltage, carrier=BEV charger, components=Link, aggregate_time=aggregate_per_year, kwargs)
n_statistics-->>FinalEnergyByCarrierElectricity: transpo Series
Note over FinalEnergyByCarrierElectricity: Final Energy|Industry|Electricity
FinalEnergyByCarrierElectricity->>n_statistics: withdrawal(carrier=industry electricity, components=Load, aggregate_time=aggregate_per_year, kwargs)
n_statistics-->>FinalEnergyByCarrierElectricity: industry Series
Note over FinalEnergyByCarrierElectricity: DAC electricity demand
FinalEnergyByCarrierElectricity->>n_statistics: withdrawal(bus_carrier=AC, carrier=DAC, components=Link, aggregate_time=aggregate_per_year, kwargs)
n_statistics-->>FinalEnergyByCarrierElectricity: dac Series
FinalEnergyByCarrierElectricity->>FinalEnergyByCarrierElectricity: series_list = [agri, rescom, transpo, industry, dac]
FinalEnergyByCarrierElectricity->>FinalEnergyByCarrierElectricity: filter out empty series from series_list
FinalEnergyByCarrierElectricity->>FinalEnergyByCarrierElectricity: result = concat(series_list)
FinalEnergyByCarrierElectricity-->>Caller: return result Series/DataFrame
deactivate FinalEnergyByCarrierElectricity
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…arrierelectricity
…tics-input to use pd.concat
Owner
Author
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…arrierelectricity
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new implementation relies on
kwargsandkwargs_filteringinFinal_Energy_by_Carrier__Electricity, but these are neither defined nor passed into the function, which will currently raise aNameErrorand should be replaced with explicit parameters or constructed locally from the function arguments. - The residential/commercial low-voltage filtering builds
forbitten_listvia nested loops and substring checks onlv.index.get_level_values('carrier'); this can be simplified and made more robust with vectorized string matching (e.g. usingSeries.str.containswith a combined pattern and a boolean mask) instead of accumulating a Python list. - In
rescom = rescom.groupby(kwargs['groupby']).sum(), the code assumes a'groupby'key inkwargs, which may not exist; consider deriving the grouping directly from the index or from explicit parameters to avoid key errors and make the grouping behavior clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new implementation relies on `kwargs` and `kwargs_filtering` in `Final_Energy_by_Carrier__Electricity`, but these are neither defined nor passed into the function, which will currently raise a `NameError` and should be replaced with explicit parameters or constructed locally from the function arguments.
- The residential/commercial low-voltage filtering builds `forbitten_list` via nested loops and substring checks on `lv.index.get_level_values('carrier')`; this can be simplified and made more robust with vectorized string matching (e.g. using `Series.str.contains` with a combined pattern and a boolean mask) instead of accumulating a Python list.
- In `rescom = rescom.groupby(kwargs['groupby']).sum()`, the code assumes a `'groupby'` key in `kwargs`, which may not exist; consider deriving the grouping directly from the index or from explicit parameters to avoid key errors and make the grouping behavior clearer.
## Individual Comments
### Comment 1
<location path="pypsa_validation_processing/statistics_functions.py" line_range="129-132" />
<code_context>
+ **kwargs,
+ )
+
+ series_list = [agri, rescom, transpo, industry, dac]
+ series_list = [series for series in series_list if not series.empty]
+
+ result = pd.concat(
+ series_list
+ ) # reduce(lambda a, b: a.add(b, fill_value=0), series_list)
</code_context>
<issue_to_address>
**issue:** Concatenating an empty `series_list` will raise a `ValueError` and may need explicit handling.
If this “all components empty” case is possible (e.g. networks with no demand in these categories), handle it explicitly by either returning an empty Series/DataFrame with the expected index/columns, or short‑circuiting before `pd.concat` so the API remains consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
pworschischek-aggmag
approved these changes
Apr 24, 2026
Collaborator
pworschischek-aggmag
left a comment
pworschischek-aggmag
left a comment
There was a problem hiding this comment.
Looks good, just some questions from my side:
- can we really negelct heat?
Also, it might be time to think about caching. the evals module uses a cache here: https://github.com/AGGM-AG/pypsa-at/blob/af8f49ea2be27e2c8c5cf34df1babb6008b91bfe/evals/views/demand.py#L297
for the expensive very fed calculations (conversion of useful energy as heat into final energy by carrier, e.g. electricity)
| BEV charging) industry electricity loads, and DAC electricity demand | ||
| change in home battery is of magnitude 1e-9 compared to electricity demand. | ||
| Concerning heat: rural air heat pump, rural ground heat pump, rural resisive | ||
| heater and urban decentral heatings are included. Central heatings using |
Collaborator
There was a problem hiding this comment.
central heat electricity demands my be relevant. Whats the reason for the exclusion?
Owner
Author
There was a problem hiding this comment.
I did not see the carrier of central heat as element of "Final Energy" and therefore excluded it, as the respective Final Energy would be heat
| pd.concat([res, res_storage.mul(-1)], axis=0) | ||
| .groupby(["location", "unit"]) | ||
| .sum() | ||
| forbitten_parts = [ |
Collaborator
There was a problem hiding this comment.
typo in varname: forbidden
| agriculture electricity loads, residential/commercial low-voltage loads | ||
| (excluding dedicated industry/agriculture/charger/distribution categories, | ||
| BEV charging) industry electricity loads, and DAC electricity demand | ||
| change in home battery is of magnitude 1e-9 compared to electricity demand. |
Collaborator
There was a problem hiding this comment.
Does this mean home battery charge cycle losses are neglected?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completely refactors existing Statistics Function for Variable
Final Energy [by Carrier]|Electricityimplementing pypsa-de logicRemaining To Dos:
Note
Implementation of tests depends on PR #20 for MockNetwork
tests/- stick to the testing-READMETest-run with existing network file
summary of variables
Summary by Sourcery
Refactor electricity final energy statistics to align with pypsa-de sectoral logic and add targeted tests for withdrawal queries and result composition.
Bug Fixes:
Enhancements:
Tests: