Skip to content

Ev visibility#63

Closed
irmatgit wants to merge 9 commits intodsgrid:mainfrom
equinor:ev_visibility
Closed

Ev visibility#63
irmatgit wants to merge 9 commits intodsgrid:mainfrom
equinor:ev_visibility

Conversation

@irmatgit
Copy link
Copy Markdown
Contributor

add ev as a end-use for the stride-viewer

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR expands STRIDE’s projection/UI support by separating EV charging into its own end-use metric, improves scenario override dataset loading, and adds a new “custom demand component” capability that injects user-defined loads into the hourly energy projection.

Changes:

  • Tag EV charging energy as a distinct end-use (metric='ev_charging') in Transportation when EV projection is enabled (dbt + test implementation).
  • Add scenario-specific dataset substitution in make_mapped_datasets to correctly load scenario overrides registered as {scenario}__{table}.
  • Introduce configurable custom demand components (new model/config fields, injection into energy_projection, and CLI commands), plus UI/CLI and regression test updates.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tests/test_project.py Adds tests for recent-project registration and scenario dataset override substitution.
tests/test_energy_projection.py Updates test-side projection logic to tag EV charging with a dedicated metric.
tests/test_custom_demand.py Adds a new test suite covering custom demand component behaviors and CLI lifecycle.
tests/test_app_cache.py Adds regression test ensuring Settings save doesn’t set the CLI override.
tests/test_api.py Relaxes assertions to allow zero values (non-negative) for annual metrics.
src/stride/ui/settings/callbacks.py Stops Settings save from setting the runtime max-projects override; persists config + evicts only.
src/stride/ui/app.py Allows passing dynamic sector lists into fresh color manager creation.
src/stride/project.py Injects custom demand rows into scenario energy projections (materializes view to table first).
src/stride/models.py Adds CustomDemandComponent, scenario-level overrides, and project-level component configuration.
src/stride/dsgrid_integration.py Substitutes scenario-specific dataset IDs when present to fix override loading.
src/stride/dbt/models/energy_projection_com_ind_tra_load_shapes.sql Adds energy_source tagging and emits EV charging as ev_charging metric.
src/stride/cli/stride.py Registers newly created projects as “recent” and adds custom-demand CLI commands.
.gitignore Adds ignores for macOS metadata and equinor*.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/stride/project.py
Comment on lines +639 to +646
# Inject custom demand components (after dbt, before copying to main)
# dbt outputs energy_projection as a view; materialize it as a table
# so we can INSERT custom demand rows into it.
if self._config.custom_demand_components:
self._con.sql(
f"CREATE TABLE {scenario.name}.__ep_tmp AS "
f"SELECT * FROM {scenario.name}.energy_projection"
)
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

PR description mentions only “EV visibility / add ev as an end-use”, but this PR also introduces a full custom-demand component system + CLI commands and settings behavior changes. If these are intended, consider updating the PR description (or splitting into separate PRs) to make review/rollback safer and scope clearer.

Copilot uses AI. Check for mistakes.
Comment thread src/stride/models.py
Comment on lines +116 to +122
custom_demand_overrides: dict[str, Path] = Field(
default={},
description=(
"Per-scenario overrides for custom demand components. "
"Keys are component names, values are paths to alternative data files."
),
)
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

custom_demand_overrides uses Field(default={}), which is a shared mutable default and can leak overrides across Scenario instances. Use default_factory=dict instead (and similarly ensure values are copied when mutating).

Copilot uses AI. Check for mistakes.
Comment thread src/stride/models.py
Comment on lines +213 to +216
custom_demand_components: list[CustomDemandComponent] = Field(
default=[],
description="Additive custom demand components (e.g., heat pumps, data centers)",
)
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

custom_demand_components is declared with Field(default=[]), which is a shared mutable default and can leak components across ProjectConfig instances. Use default_factory=list (and consider aligning other list defaults in this model similarly).

Copilot uses AI. Check for mistakes.
Comment thread src/stride/project.py
Comment on lines +741 to +746
# Defensive DELETE: remove any existing rows for this custom component
self._con.sql(
f"DELETE FROM {scenario.name}.energy_projection "
f"WHERE sector = '{component.sector}'"
f" AND metric = '{component.metric}'"
)
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

The DELETE statement interpolates component.sector/component.metric directly into SQL. This will break for values containing quotes and is also SQL-injection prone since these fields come from user config/CLI. Use parameterized queries (DuckDB ? placeholders) rather than f-string interpolation here.

Copilot uses AI. Check for mistakes.
Comment thread src/stride/project.py
Comment on lines +741 to +746
# Defensive DELETE: remove any existing rows for this custom component
self._con.sql(
f"DELETE FROM {scenario.name}.energy_projection "
f"WHERE sector = '{component.sector}'"
f" AND metric = '{component.metric}'"
)
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

Deleting by only (sector, metric) can wipe out baseline projection rows if a user chooses an existing sector/metric (e.g., Residential/heating), turning an “additive” component into destructive overwrite. Consider preventing collisions (validate against existing sector/metric pairs) or tagging injected rows with a component identifier so DELETE targets only the injected component.

Suggested change
# Defensive DELETE: remove any existing rows for this custom component
self._con.sql(
f"DELETE FROM {scenario.name}.energy_projection "
f"WHERE sector = '{component.sector}'"
f" AND metric = '{component.metric}'"
)
# Prevent destructive overwrite of baseline rows by rejecting
# custom components that collide with an existing sector/metric pair.
existing_count = self._con.sql(
f"SELECT COUNT(*) FROM {scenario.name}.energy_projection "
f"WHERE sector = '{component.sector}'"
f" AND metric = '{component.metric}'"
).fetchone()[0]
if existing_count > 0:
msg = (
f"Custom demand component '{component.name}' uses sector "
f"'{component.sector}' and metric '{component.metric}', "
f"which already exist in scenario '{scenario.name}' "
f"energy_projection data. Choose a unique sector/metric "
f"pair for additive custom demand components."
)
raise InvalidParameter(msg)

Copilot uses AI. Check for mistakes.
Comment thread src/stride/project.py
Comment on lines +869 to +883
hourly_timestamps AS (
SELECT DISTINCT timestamp, model_year
FROM {scenario_name}.energy_projection
)
SELECT
ht.timestamp,
ht.model_year,
'{self._config.country}' AS geography,
'{component.sector}' AS sector,
'{component.metric}' AS metric,
ad.annual_mwh / 8760.0 AS value,
'{scenario_name}' AS scenario
FROM hourly_timestamps ht
JOIN annual_data ad ON ht.model_year = ad.model_year
"""
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

Flat profile injection divides by a hard-coded 8760 hours, but energy_projection timestamps can be 8784 for leap weather years (and could differ for other calendars). This will cause annual totals to be wrong. Compute the per-year hour count from hourly_timestamps and divide by that instead.

Copilot uses AI. Check for mistakes.
Comment thread src/stride/project.py
Comment on lines +1051 to +1057
if row_count != 8760:
msg = (
f"Custom profile file must have exactly 8760 rows "
f"(got {row_count})"
)
raise InvalidParameter(msg)

Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

File-profile injection hard-requires exactly 8760 rows, but energy_projection can have a different number of hourly timestamps (e.g., 8784 for leap years). Validate the profile length against the per-year timestamp count you will join to (and/or support 8784), otherwise joins will silently drop/mismatch hours.

Suggested change
if row_count != 8760:
msg = (
f"Custom profile file must have exactly 8760 rows "
f"(got {row_count})"
)
raise InvalidParameter(msg)
timestamp_counts = self._con.sql(
f"""
SELECT model_year, COUNT(DISTINCT timestamp) AS timestamp_count
FROM {scenario_name}.energy_projection
WHERE model_year IN ({model_years_str})
GROUP BY model_year
ORDER BY model_year
"""
).fetchall()
if not timestamp_counts:
msg = (
"No hourly timestamps found in energy_projection for the "
f"selected model years: {model_years_str}"
)
raise InvalidParameter(msg)
expected_counts = {count for _, count in timestamp_counts}
if len(expected_counts) != 1:
counts_by_year = ", ".join(
f"{model_year}: {count}" for model_year, count in timestamp_counts
)
msg = (
"Custom profile file cannot be applied because the selected "
"model years do not all have the same number of hourly "
f"timestamps in energy_projection. Found: {counts_by_year}"
)
raise InvalidParameter(msg)
expected_row_count = next(iter(expected_counts))
if row_count != expected_row_count:
msg = (
"Custom profile file must have exactly one row per hourly "
"timestamp in energy_projection for each selected model year "
f"(expected {expected_row_count}, got {row_count})"
)
raise InvalidParameter(msg)

Copilot uses AI. Check for mistakes.
Comment thread src/stride/project.py
Comment on lines +896 to +899
count = self._con.sql(
f"SELECT COUNT(*) FROM {scenario_name}.load_shapes_expanded "
f"WHERE {dimension} = '{ref_value}' "
f"AND model_year IN ({model_years_str})"
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

_validate_load_shape_reference interpolates ref_value directly into SQL. A reference containing quotes will break the query, and since load_profile is user-provided this is also injection-prone. Use parameter binding for ref_value (and avoid string formatting values into SQL).

Suggested change
count = self._con.sql(
f"SELECT COUNT(*) FROM {scenario_name}.load_shapes_expanded "
f"WHERE {dimension} = '{ref_value}' "
f"AND model_year IN ({model_years_str})"
count = self._con.execute(
f"SELECT COUNT(*) FROM {scenario_name}.load_shapes_expanded "
f"WHERE {dimension} = ? "
f"AND model_year IN ({model_years_str})",
[ref_value],

Copilot uses AI. Check for mistakes.
@irmatgit
Copy link
Copy Markdown
Contributor Author

PR on the wrong repo..

@irmatgit irmatgit closed this Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants