feat(scenario aggregation): introduce new feature scenario aggregation - #90
feat(scenario aggregation): introduce new feature scenario aggregation#90dusanparipovic wants to merge 5 commits into
Conversation
| metric_structure_table = self.metric_structure_table_builder.build(metric) | ||
| metric_view = self.terms_aggregator.run(metric_structure_table, metric) | ||
| temporal_metric_view = self.time_aggregator.run(metric_view, metric) | ||
| self.scenario_aggregator.run(temporal_metric_view) |
There was a problem hiding this comment.
Is there a specific reason why scenario_aggregator modify a MetricView, where a time_aggregator returns one ? This is does not look very homogeneous.
There was a problem hiding this comment.
As you can see on line 43, we append temporal_metric_view to the metric_views list.
We don't have any strict requirements for the MetricView object. In practice, it only stores the path to the temporary view.
If you look at the run method of the ScenarioAggregator, you'll notice that it doesn't return anything. That's because I intentionally rely on Python's reference semantics: the temporal_metric_view object is passed by reference, and the aggregator updates its content (specifically, the path stored in the MetricView object).
From my perspective, there's no need to create a new object with identical data. We can simply update the existing object instead.
There was a problem hiding this comment.
Add a comment in the beginning of the function to make the behavior difference explicit :
"""Rewrites temporal_metric_view's parquet file in place (same path, new content)
and returns the same MetricView instance; it does not produce a separate artifact.""
| # legacy | ||
| # metric_id | metric_location | breakdown_property | view_date | scenario_id | metric_value | |
There was a problem hiding this comment.
Please remove Legacy, then :)
|
Suggestion for try:
... # scan_parquet / sink_parquet
except Exception:
try:
os.remove(tmp_path)
except FileNotFoundError:
pass
raiseWhy: |
|
Please add something to this PR's description :
Otherwise, there are things that the reader may have to guess. |
This PR enables scenario aggregation in the views, and always computes the min, max, expectation and standard deviation indicators over all scenarios.
Example:
view_config.ymlwith scenario aggregation enabledThat's it on the config side — same shape as before, just
scenario: trueinstead offalse/omitted.load_view_config()reads it intoViewConfig.scenario_aggregation = True(view_config.py:127-129), andViewBuilder.__init__constructsScenarioAggregator(True)from it (views_builder.py:34).Resulting build behavior
For each metric,
ViewBuilder.build()still runsTermsAggregator→TimeAggregatorto get atemporal_metric_viewwith per-scenario rows keyed onmetric_id, metric_location, breakdown_properties, scenario_id, view_date. ThenScenarioAggregator.run()(scenario_aggregator.py:34-58) takes over and rewrites that parquet in place:metric_id, metric_location, breakdown_properties, view_date—scenario_idis dropped from the group key, so all scenarios for a given (metric, location, breakdown, timestamp) are collapsed into one group.metric_valuewith all fourSCENARIO_AGG_EXPRS:exp=mean(metric_value)std=std(metric_value, ddof=0)(population std — always defined,0for a single-scenario view, no null edge case)min=min(metric_value)max=max(metric_value)scenario_statand its value intometric_value.scenario_id = None(a synthesis row no longer corresponds to a single scenario) andscenario_aggregation = True.So for one
(metric_id, metric_location, breakdown_properties, view_date)combination that previously had, say, 10 rows (one per scenario 0–9), the built view now has exactly 4 rows:nulltrueexpnulltruestdnulltrueminnulltruemaxBy contrast, with
scenario: false(or omitted), the row count per group is unchanged from today — one row per original scenario — but every row now also carriesscenario_aggregation = falseandscenario_stat = null, so the schema (metric_id | metric_location | breakdown_properties | view_date | scenario_id | scenario_aggregation | scenario_stat | metric_value) stays identical across both modes; only the row semantics differ.