Skip to content

fix: restore correct assigned countries for virtual tyndp nodes - #794

Merged
daniel-rdt merged 5 commits into
masterfrom
fix/itco-country
Jul 23, 2026
Merged

fix: restore correct assigned countries for virtual tyndp nodes#794
daniel-rdt merged 5 commits into
masterfrom
fix/itco-country

Conversation

@daniel-rdt

@daniel-rdt daniel-rdt commented Jul 17, 2026

Copy link
Copy Markdown
Member

Closes #765.

Changes proposed in this Pull Request

This PR introduces a minor bug fix that wrongly assigned the country for the ITCO virtual node to FR instead of IT as manually set in build_tyndp_network.

The PR restores the declared country of TYNDP virtual nodes in base_network. As their coordinates are borrowed from a bus in a geographically different country, the coordinate-based assignment in _set_countries_and_substations is incorrectly reassigning them. buses holds the countries declared upstream in build_tyndp_network, before that reassignment and can be used to restore the correct value.

Note: The help of generative AI was used to generate docstrings for the introduced functions

Tasks

Workflow

Open issues

Notes

Checklist

Required:

  • Security scans show no high-severity bugs, critical vulnerabilities, or exposed secrets.
  • Changes are tested locally and behave as expected.
  • Code and workflow changes are documented.
  • A release note entry is added to doc/release_notes.md.
  • The description is human-written and any AI-generated content is marked.

If applicable:

  • Changes in configuration options are reflected in scripts/lib/validation.
  • Changes in configuration options are added to config/test/*.yaml.
  • Multiple climate years test passes locally (pixi run -e open-tyndp tyndp-cyears-test).
  • For new data sources or versions, these instructions have been followed.
  • Open-TYNDP SPDX license header is added to all touched files.
  • Module docstrings are added to new Python scripts.
  • New rules are documented in the appropriate doc/*.md files.
  • Major features are documented in doc/index.md.

@daniel-rdt daniel-rdt added this to the Release v0.7.2 milestone Jul 17, 2026
@daniel-rdt daniel-rdt self-assigned this Jul 17, 2026
@daniel-rdt daniel-rdt added SB Scenario Building CBA Cost Benefit Analysis labels Jul 17, 2026
@daniel-rdt
daniel-rdt requested a review from tgilon July 17, 2026 11:18
@daniel-rdt
daniel-rdt marked this pull request as ready for review July 17, 2026 11:18

@tgilon tgilon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @daniel-rdt for the fix. I think we could generalise a bit more to effectively link the two scripts via the global variable. Currently, the link is probably too weak to be robust.

Comment thread scripts/build_tyndp_network.py Outdated
"UK": "GB",
}

AC_VIRTUAL_NODES = ["ITCO", "ITVI"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I understand the need to link the two files. You could create a dictionary instead and use that when adding the virtual nodes. This would be more robust.

@daniel-rdt

Copy link
Copy Markdown
Member Author

Thanks @tgilon I refactored and generalised the addition of the virtual nodes in build_tyndp_network using a helper function and turning the global variable into a mapping dict.

Currently CI is failing due to the collect vs combine bug fixed in #797

@daniel-rdt
daniel-rdt requested a review from tgilon July 22, 2026 07:54

@tgilon tgilon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @daniel-rdt ! I think that this is more consistent internally. I have one final minor comment for the docstring. Otherwise, you could base this PR on #797 to solve the CI issue.

Comment thread scripts/build_tyndp_network.py Outdated
ref_bus : str
Name of the reference bus whose attributes are copied.
source_gdf : gpd.GeoDataFrame, optional
Bus GeoDataFrame holding the reference bus. Defaults to ``target``.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The default is None, which implies the use of target_df

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

jep, good point. I clarified it.

@tgilon

tgilon commented Jul 22, 2026

Copy link
Copy Markdown
Member

#797 has now been merged on master

@daniel-rdt
daniel-rdt merged commit 32ff08a into master Jul 23, 2026
9 of 10 checks passed
@daniel-rdt
daniel-rdt deleted the fix/itco-country branch July 23, 2026 13:07
@tgilon tgilon modified the milestones: Release v0.7.2, Release v0.8 Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CBA Cost Benefit Analysis SB Scenario Building

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong country defined for ITCO bus

2 participants