Skip to content

Commit

Permalink
fix: delta profile when 2 result sets have different periods
Browse files Browse the repository at this point in the history
due to opposite check of what it was supposed to, ie. check that
we only include periods that are given in the common period vector,
but instead there was a check that the separate result had a
period that was not in the common period vector.

Resolves equinor/ecalc-internal#298
  • Loading branch information
TeeeJay committed Dec 17, 2024
1 parent 56f60db commit 2dd705f
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
3 changes: 2 additions & 1 deletion src/libecalc/common/time_utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import enum
from collections.abc import Iterator
from dataclasses import dataclass
from datetime import date, datetime, timedelta
from typing import Any, Optional, Self, Union
Expand Down Expand Up @@ -160,7 +161,7 @@ def create_periods(times: list[datetime], include_before: bool = True, include_a

return Periods(periods=periods)

def __iter__(self):
def __iter__(self) -> Iterator[Period]:
return self.periods.__iter__()

def get_period(self, period: Period) -> Optional[Period]:
Expand Down
15 changes: 9 additions & 6 deletions src/libecalc/presentation/simple_result/simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def fit_to_periods(
delta profile comparisons easier/possible.
Args:
component (SimpleComponentResult): The component that should be fitted.
component (SimpleComponentResult): The component that should be fitted (should intersect all periods in the periods list).
periods (Periods): The target periods. The provided periods should all exist in the component.
Returns:
Expand All @@ -145,7 +145,7 @@ def fit_to_periods(
emission.name: SimpleEmissionResult(name=emission.name, rate=[])
for emission in component.emissions.values()
}
for period_index, _period in enumerate(component.periods):
for period_index, _period in enumerate(periods):
period = Period.intersection(_period, periods.period)
if period:
start = periods.start_dates.index(period.start)
Expand All @@ -159,7 +159,8 @@ def fit_to_periods(
# Assume index exist if emission exist
emission.rate.append(component.emissions[emission.name].rate[period_index])
else:
# This is a developer error, we should provide the correct period.
# It is a developer error if the component does not contain a period it should have according
# to the periods provided.
raise ProgrammingError(
f"Provided periods includes period not found in component {component.id}. "
f"Extraneous period: {period}. This should not happen, contact support."
Expand Down Expand Up @@ -256,7 +257,7 @@ def fit_to_periods(
cls,
model: "SimpleResultData",
periods: Periods,
):
) -> "SimpleResultData":
"""
Fit result to periods. Only a subset or the same set of periods is supported.
Args:
Expand Down Expand Up @@ -323,7 +324,7 @@ def normalize_components(
reference_model: "SimpleResultData",
changed_model: "SimpleResultData",
exclude: Optional[list[ComponentType]] = None,
):
) -> tuple["SimpleResultData", "SimpleResultData"]:
if exclude is None:
exclude = []

Expand Down Expand Up @@ -379,12 +380,14 @@ def delta_profile(
first_date = max(changed_model.periods.first_date, reference_model.periods.first_date)
last_date = min(changed_model.periods.last_date, reference_model.periods.last_date)

# intersection of the dates in both models
all_dates_in_models = sorted(
{date for date in reference_model.periods.all_dates if first_date <= date <= last_date}.union(
{date for date in changed_model.periods.all_dates if first_date <= date <= last_date}
)
)
# define new periods using all dates in both models

# define new periods using all dates in both models, skip extra periods before and after
periods = Periods.create_periods(
times=all_dates_in_models,
include_after=False,
Expand Down

0 comments on commit 2dd705f

Please sign in to comment.