Skip to content

feat: add new types of loads for reeds to plexos translations#256

Open
mcllerena wants to merge 16 commits into
mainfrom
ml/add-new-load
Open

feat: add new types of loads for reeds to plexos translations#256
mcllerena wants to merge 16 commits into
mainfrom
ml/add-new-load

Conversation

@mcllerena
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings May 6, 2026 23:52
Copy link
Copy Markdown
Contributor

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for translating new ReEDS demand component types (electrolyzer + data center) into PLEXOS Purchasers, including purchaser/node memberships and time series transfer.

Changes:

  • Add rules mapping ReEDSElectrolyzerDemand and ReEDSDataCenterDemand to PLEXOSPurchaser and add purchaser→node membership creation.
  • Add purchaser time series transfer step to the ReEDS→PLEXOS translation flow.
  • Update tests to cover rule presence and translation behavior for the new demand types.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
pyproject.toml Adds new dependencies and switches several uv sources to editable path-based deps.
packages/r2x-reeds-to-plexos/tests/test_translation_rule_application.py Adds end-to-end translation assertions for electrolyzer/data-center demands.
packages/r2x-reeds-to-plexos/tests/test_rule_loading.py Adds tests ensuring new mapping rules exist in rules.json.
packages/r2x-reeds-to-plexos/tests/test_getters_utils.py Adds unit test for attaching time series to purchasers.
packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/translation.py Runs purchaser time-series attachment during translation.
packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/getters_utils.py Implements attach_time_series_to_purchasers.
packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/getters.py Expands lookup to include new demand components for memberships.
packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/config/rules.json Adds new purchaser rules + purchaser node membership; adjusts consuming-tech rule filtering.
packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/config/defaults.json Adds an electrolyzer defaults block.
packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/init.py Exports attach_time_series_to_purchasers.

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

Comment thread packages/r2x-reeds-to-plexos/tests/test_getters_utils.py Outdated
Comment thread pyproject.toml
Comment thread pyproject.toml Outdated
Comment thread packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/getters.py Outdated
Comment thread packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/getters.py Outdated
Comment thread packages/r2x-reeds-to-plexos/src/r2x_reeds_to_plexos/getters_utils.py Outdated
@github-actions github-actions Bot removed the r2x label May 7, 2026
@mcllerena mcllerena requested a review from pesap May 21, 2026 04:23
@github-actions github-actions Bot added Docs Improvements or additions to documentation r2x labels May 23, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 3, 2026

Codecov Report

❌ Patch coverage is 87.50000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.99%. Comparing base (649eba0) to head (7f49f37).

Files with missing lines Patch % Lines
...to-plexos/src/r2x_reeds_to_plexos/getters_utils.py 89.18% 4 Missing ⚠️
...reeds-to-plexos/src/r2x_reeds_to_plexos/getters.py 76.92% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #256      +/-   ##
==========================================
+ Coverage   84.95%   84.99%   +0.03%     
==========================================
  Files          19       19              
  Lines        4561     4584      +23     
==========================================
+ Hits         3875     3896      +21     
- Misses        686      688       +2     
Files with missing lines Coverage Δ
...-to-sienna/src/r2x_plexos_to_sienna/translation.py 100.00% <100.00%> (ø)
...eeds-to-plexos/src/r2x_reeds_to_plexos/__init__.py 100.00% <ø> (ø)
...s-to-plexos/src/r2x_reeds_to_plexos/translation.py 100.00% <100.00%> (ø)
...s-to-sienna/src/r2x_reeds_to_sienna/translation.py 100.00% <100.00%> (ø)
...enna-to-plexos/src/r2x_sienna_to_plexos/getters.py 81.96% <100.00%> (-0.02%) ⬇️
...-to-plexos/src/r2x_sienna_to_plexos/translation.py 100.00% <100.00%> (ø)
...reeds-to-plexos/src/r2x_reeds_to_plexos/getters.py 83.05% <76.92%> (+0.47%) ⬆️
...to-plexos/src/r2x_reeds_to_plexos/getters_utils.py 92.63% <89.18%> (-1.57%) ⬇️

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@pesap pesap left a comment

Choose a reason for hiding this comment

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

Requesting changes for the dependency contract issue noted inline.

"units": "get_component_units"
},
"source_type": "ReEDSElectrolyzerDemand",
"target_type": "PLEXOSPurchaser",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One blocker: the package metadata still allows dependency versions that do not provide the new model classes used by these rules.

Evidence:

  • packages/r2x-reeds-to-plexos/pyproject.toml allows r2x-reeds>=0.4.0,<1.0.0 and r2x-plexos>=0.1.4,<1.0.0.
  • The new rules reference ReEDSElectrolyzerDemand, ReEDSDataCenterDemand, and PLEXOSPurchaser.
  • r2x-reeds==0.4.0 does not expose the new ReEDS demand classes, and r2x-plexos==0.1.4 does not expose PLEXOSPurchaser.

Can you either bump the minimum dependency versions to released versions that contain those classes, or make the new rules conditional on class availability? Otherwise valid installs under the declared package contract can fail the new translation path.


# Test node getters
assert getters.get_node_number(node, context).unwrap() == 123
assert getters.get_node_number(node, context).unwrap() == 1230
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This assertion appears to encode test-order dependence. get_node_number("p123") earlier in the module reserves 123 in global state, so NODE_123 becomes 1230 only because a previous test ran first. Please reset the global numbering state between tests or move numbering state into the translation context, then keep NODE_123 deterministic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants