Add discrete capacity extension#345
Open
idelder wants to merge 18 commits into
Open
Conversation
WalkthroughIntroduces a declarative extension framework ( ChangesExtension Framework
Existing/Retired Capacity and Lifetime Refactor
Tutorial Database Schema-First Generation
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant Sequencer
participant build_instance
participant TemoaModel
participant HybridLoader
Sequencer->>build_instance: extensions=config.extensions
build_instance->>TemoaModel: TemoaModel(extensions=...)
TemoaModel->>TemoaModel: apply_model_extension_hooks
Sequencer->>HybridLoader: build manifest with extension_ids
HybridLoader->>TemoaModel: resolve_extension_specs
sequenceDiagram
participant CLI
participant SQLiteConn
CLI->>SQLiteConn: execute temoa_schema_v4.sql
CLI->>SQLiteConn: PRAGMA foreign_keys = OFF
CLI->>SQLiteConn: execute utopia.sql
CLI->>SQLiteConn: PRAGMA foreign_keys = ON
CLI->>SQLiteConn: PRAGMA foreign_key_check
SQLiteConn-->>CLI: violations or none
CLI->>SQLiteConn: commit
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
idelder
force-pushed
the
extension/integer_capacity
branch
from
bff1c69 to
9cf073b
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
idelder
force-pushed
the
extension/integer_capacity
branch
from
9cf073b to
879e411
Compare
updates: - [github.com/astral-sh/uv-pre-commit: 0.11.19 → 0.11.21](astral-sh/uv-pre-commit@0.11.19...0.11.21) - [github.com/astral-sh/ruff-pre-commit: v0.15.16 → v0.15.17](astral-sh/ruff-pre-commit@v0.15.16...v0.15.17)
updates: - [github.com/astral-sh/uv-pre-commit: 0.11.21 → 0.11.23](astral-sh/uv-pre-commit@0.11.21...0.11.23) - [github.com/astral-sh/ruff-pre-commit: v0.15.17 → v0.15.18](astral-sh/ruff-pre-commit@v0.15.17...v0.15.18)
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
…counting purposes Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
…ly retirement, and growth rate constraints all together Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
…capacities Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
idelder
force-pushed
the
extension/integer_capacity
branch
from
879e411 to
6b21b35
Compare
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
idelder
force-pushed
the
extension/integer_capacity
branch
from
6b21b35 to
9c61cf6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/extensions/growth_rates.rst`:
- Around line 101-199: The docs page is duplicated starting in the second half,
which will render all sections twice and reintroduce the same
_extension-growth-rates label collision. Remove the repeated block from the
growth_rates.rst content after the first complete Constraints section, keeping
only one copy of the extension overview, Parameters table, per-parameter
subsections, and autofunction references.
In `@temoa/data_io/hybrid_loader.py`:
- Around line 654-657: Initialize viable_existing_techs in __init__ alongside
viable_existing_rt and viable_existing_rtv, since it is currently only set
inside _load_existing_capacity when rows_to_load is present. Add the same
empty-set default there so the HybridLoader instance always has
viable_existing_techs defined before any reader accesses it, even when no
existing capacity data is loaded.
- Around line 704-711: The optional lifetime loader path in _load_lifetime_tech
still queries lifetime_tech directly even though its manifest entry is not
required, so a missing table can raise OperationalError instead of being
skipped. Add a table_exists(...) check in _load_lifetime_tech before opening the
cursor and fetching rows, following the same pattern used by
_load_retired_existing_capacity, and apply the same guard to
_load_lifetime_process and _load_lifetime_survival_curve so all three optional
loaders safely no-op when their tables are absent.
In `@temoa/extensions/growth_rates/components/growth_capacity.py`:
- Around line 93-98: The `periods` set in `growth_capacity.py` is still being
derived from the full `cap_rpt` map, so `Constraint.Skip` only happens when
everything is empty instead of when the filtered region/tech slice is empty.
Update the `growth_capacity` constraint logic to build `periods` from
`cap_indices` rather than `cap_rpt`, keeping the empty-slice behavior aligned
with the sibling rules that already use the filtered indices.
In `@temoa/extensions/integer_capacity/components/integer_capacity.py`:
- Around line 18-26: The integer-capacity index builders are generating unused
(region, technology, vintage/period) combinations, which creates unnecessary
variables and constraints. Update limit_integer_new_capacity_indices and
limit_integer_net_capacity_indices so they only return indices that actually
exist in model.process_periods and model.v_capacity_available_by_period_and_tech
before creating the Set. Keep the filtering in the constraint rules as a
safeguard, but avoid building the full cross-product in these helper functions.
In `@temoa/extensions/integer_capacity/core/model.py`:
- Around line 28-30: The capacity unit-size Params in IntegerCapacityModel are
currently unvalidated because limit_integer_new_capacity and
limit_integer_net_capacity use the default Any domain, so bad database values
can slip into the constraint logic. Update the Param declarations in the model
setup to use an explicit numeric domain such as PositiveReals, or
NonNegativeReals if zero is allowed, so invalid values are caught during model
construction. Apply the same domain change consistently to both parameters
referenced in the integer capacity constraint methods.
In `@temoa/extensions/template/components/example_limit.py`:
- Around line 18-19: Update the imports in example_limit.py to use a direct
grouped import from temoa.components instead of aliasing the modules as
geography and technology, and adjust the new_cap_rtv loop to iterate over the
dict directly rather than calling keys(). Use the existing geography/technology
references and the new_cap_rtv iteration site to locate the cleanup, and apply
the same dict-iteration fix wherever the same pattern appears in the file.
- Around line 51-56: The constraint in the example_limit template is doing a
full scan of model.v_new_capacity.keys() inside each rule evaluation, which
causes repeated O(index × keyspace) work. Update the logic around the new_cap
calculation to precompute a lookup or grouped structure for v_new_capacity
before the constraint rule runs, then have the rule use that cached grouping
instead of iterating all keys each time. Use the example_limit template’s
new_cap and new_cap_rtv block as the place to refactor.
In `@temoa/extensions/template/core/model.py`:
- Around line 58-60: The example parameter declaration for
example_new_capacity_limit is too permissive because it uses Any even though the
value is meant to be a numeric cap. Update the Param definition in the template
model to use an appropriate numeric domain such as NonNegativeReals so derived
extensions follow best practice and reject non-numeric values; keep the change
localized to the example_new_capacity_limit symbol.
In `@temoa/extensions/template/extension.py`:
- Line 30: The path construction in the extension setup should use the more
idiomatic Path parent accessor instead of indexing into parents. Update the
schema_sql_path assignment in the extension.py initialization code to use
Path(__file__).parent when building the tables.sql path, keeping the same
behavior while simplifying the expression.
In `@temoa/tutorial_assets/config_sample.toml`:
- Line 23: The example in config_sample.toml references an extension id that is
not registered, so update the commented extensions example to use only ids that
are currently available. Adjust the sample near the extensions example so it
matches the known extension ids resolved by resolve_extension_specs, such as
growth_rates and integer_capacity, and avoid mentioning unit_commitment unless
it is actually registered.
In `@tests/test_extensions.py`:
- Around line 63-79: The table-name assertion in
test_extension_schema_matches_owned_tables is too permissive because it matches
substrings and can hide missing CREATE TABLE entries. Update the check so each
owned table is verified with a boundary-aware match against schema_sql, using
the existing test_extension_schema_matches_owned_tables logic and
spec.owned_tables to locate the relevant table definitions without allowing
prefix collisions like limit_growth_new_capacity versus
limit_growth_new_capacity_delta.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eaa1c908-0d39-461b-b23e-e07dcecd2520
📒 Files selected for processing (69)
.pre-commit-config.yamldocs/source/computational_implementation.rstdocs/source/extensions.rstdocs/source/extensions/growth_rates.rstdocs/source/extensions/integer_capacity.rstdocs/source/index.rstdocs/source/mathematical_formulation.rstdocs/source/param_desc_and_tables.rsttemoa/_internal/run_actions.pytemoa/_internal/temoa_sequencer.pytemoa/cli.pytemoa/components/capacity.pytemoa/components/limits.pytemoa/components/technology.pytemoa/components/time.pytemoa/components/utils.pytemoa/core/config.pytemoa/core/model.pytemoa/data_io/component_manifest.pytemoa/data_io/hybrid_loader.pytemoa/data_io/loader_manifest.pytemoa/db_schema/temoa_schema_v4.sqltemoa/extensions/framework.pytemoa/extensions/growth_rates/__init__.pytemoa/extensions/growth_rates/components/__init__.pytemoa/extensions/growth_rates/components/growth_capacity.pytemoa/extensions/growth_rates/components/growth_new_capacity.pytemoa/extensions/growth_rates/components/growth_new_capacity_delta.pytemoa/extensions/growth_rates/core/__init__.pytemoa/extensions/growth_rates/core/model.pytemoa/extensions/growth_rates/data_manifest.pytemoa/extensions/growth_rates/extension.pytemoa/extensions/growth_rates/tables.sqltemoa/extensions/integer_capacity/__init__.pytemoa/extensions/integer_capacity/components/__init__.pytemoa/extensions/integer_capacity/components/integer_capacity.pytemoa/extensions/integer_capacity/core/__init__.pytemoa/extensions/integer_capacity/core/model.pytemoa/extensions/integer_capacity/data_manifest.pytemoa/extensions/integer_capacity/extension.pytemoa/extensions/integer_capacity/tables.sqltemoa/extensions/method_of_morris/morris.pytemoa/extensions/method_of_morris/morris_evaluate.pytemoa/extensions/modeling_to_generate_alternatives/mga_sequencer.pytemoa/extensions/myopic/myopic_sequencer.pytemoa/extensions/single_vector_mga/sv_mga_sequencer.pytemoa/extensions/stochastics/scenario_creator.pytemoa/extensions/template/__init__.pytemoa/extensions/template/components/__init__.pytemoa/extensions/template/components/example_limit.pytemoa/extensions/template/core/__init__.pytemoa/extensions/template/core/model.pytemoa/extensions/template/data_manifest.pytemoa/extensions/template/extension.pytemoa/extensions/template/tables.sqltemoa/tutorial_assets/config_sample.tomltemoa/tutorial_assets/utopia.sqltests/conftest.pytests/legacy_test_values.pytests/test_cli.pytests/test_extensions.pytests/test_full_runs.pytests/testing_configs/config_myopic_capacities.tomltests/testing_data/mediumville.sqltests/testing_data/mediumville_sets.jsontests/testing_data/myopic_capacities.sqltests/testing_data/test_system_sets.jsontests/testing_data/utopia_sets.jsontests/testing_data/utopia_v3.sql
💤 Files with no reviewable changes (4)
- docs/source/param_desc_and_tables.rst
- tests/testing_data/mediumville.sql
- docs/source/mathematical_formulation.rst
- temoa/db_schema/temoa_schema_v4.sql
idelder
force-pushed
the
extension/integer_capacity
branch
from
9c61cf6 to
4855a98
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/extensions/integer_capacity.rst`:
- Around line 31-32: The table notes in the integer capacity documentation use
“forced to an integer multiple” but should match the prose and say “forced to be
an integer multiple.” Update the descriptive text for both
limit_integer_new_capacity and limit_integer_net_capacity entries in the table
so the phrasing is grammatically consistent with the later sections.
In `@temoa/extensions/integer_capacity/components/integer_capacity.py`:
- Line 51: The integer capacity constraint is converting mutable Params to plain
numbers too early via value() in the capacity expression. In
integer_capacity.py, update the constraint-building logic in the relevant
rule(s) around the unit_cap/limit_integer_new_capacity and
limit_integer_net_capacity references to use the Param objects directly instead
of eager value() extraction, so the expressions remain symbolic and reflect
updated parameter values when the model is reused.
In `@temoa/extensions/integer_capacity/tables.sql`:
- Around line 1-16: The composite primary key in the limit_integer_new_capacity
and limit_integer_net_capacity table definitions does not enforce
non-nullability on region and tech_or_group. Update both CREATE TABLE statements
so the primary-key columns are declared NOT NULL alongside the PRIMARY KEY
constraint, using the existing table definitions as the place to fix it.
- Line 4: The integer-capacity schema currently allows zero or negative unit
sizes, which can silently create a degenerate model instead of failing fast.
Update the table definition in tables.sql for the capacity field to enforce a
positive value with a DB-level CHECK constraint, and make sure the related
integer-capacity loading path in core/model.py remains consistent with that
assumption.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f808121-8741-4d41-b05b-6a8f8b647754
📒 Files selected for processing (13)
docs/source/extensions.rstdocs/source/extensions/integer_capacity.rsttemoa/extensions/framework.pytemoa/extensions/growth_rates/components/growth_capacity.pytemoa/extensions/integer_capacity/__init__.pytemoa/extensions/integer_capacity/components/__init__.pytemoa/extensions/integer_capacity/components/integer_capacity.pytemoa/extensions/integer_capacity/core/__init__.pytemoa/extensions/integer_capacity/core/model.pytemoa/extensions/integer_capacity/data_manifest.pytemoa/extensions/integer_capacity/extension.pytemoa/extensions/integer_capacity/tables.sqltests/test_extensions.py
3 tasks
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
idelder
force-pushed
the
extension/integer_capacity
branch
from
4855a98 to
bdfa92b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
temoa/extensions/discrete_capacity/tables.sql (1)
1-16: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
CHECK(capacity > 0)to prevent degenerate zero-unit-size models.Both tables allow
capacity = 0(default), which combined with theNonNegativeRealsPyomo domain and the equality constraints incomponents/discrete_capacity.py, silently forcesnew_capacity/net_capacityto zero instead of failing on bad input. This is the same issue flagged on the predecessorinteger_capacity/tables.sqland remains unresolved after the rename todiscrete_capacity.🛡️ Proposed fix
CREATE TABLE IF NOT EXISTS limit_discrete_new_capacity( region TEXT NOT NULL, tech_or_group TEXT NOT NULL, - capacity REAL NOT NULL DEFAULT 0, + capacity REAL NOT NULL DEFAULT 0 CHECK(capacity > 0), units TEXT, notes TEXT, PRIMARY KEY (region, tech_or_group) ); CREATE TABLE IF NOT EXISTS limit_discrete_capacity( region TEXT NOT NULL, tech_or_group TEXT NOT NULL, - capacity REAL NOT NULL DEFAULT 0, + capacity REAL NOT NULL DEFAULT 0 CHECK(capacity > 0), units TEXT, notes TEXT, PRIMARY KEY (region, tech_or_group) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@temoa/extensions/discrete_capacity/tables.sql` around lines 1 - 16, Add a table-level CHECK constraint in both discrete capacity table definitions so `capacity` cannot be zero or negative. Update the `CREATE TABLE IF NOT EXISTS` statements in `tables.sql` for `limit_discrete_new_capacity` and `limit_discrete_capacity` to enforce `capacity > 0`, matching the intended behavior already implied by `components/discrete_capacity.py` and preventing zero-sized models from being accepted silently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@temoa/extensions/discrete_capacity/components/discrete_capacity.py`:
- Line 51: The discrete capacity constraint setup is allowing zero-sized units,
which can collapse the equality and disable the affected technology or group. In
the discrete_capacity component, update the logic around unit_cap in the
capacity constraint construction so that limit_discrete_new_capacity and
limit_discrete_capacity are required to be strictly positive before being used.
Add a guard or validation in the relevant constraint-building path so zero
values are rejected rather than producing a zero equality.
---
Duplicate comments:
In `@temoa/extensions/discrete_capacity/tables.sql`:
- Around line 1-16: Add a table-level CHECK constraint in both discrete capacity
table definitions so `capacity` cannot be zero or negative. Update the `CREATE
TABLE IF NOT EXISTS` statements in `tables.sql` for
`limit_discrete_new_capacity` and `limit_discrete_capacity` to enforce `capacity
> 0`, matching the intended behavior already implied by
`components/discrete_capacity.py` and preventing zero-sized models from being
accepted silently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 309a38ed-a69d-4b99-9bdc-a34c33c693f4
📒 Files selected for processing (13)
docs/source/extensions.rstdocs/source/extensions/discrete_capacity.rsttemoa/extensions/discrete_capacity/__init__.pytemoa/extensions/discrete_capacity/components/__init__.pytemoa/extensions/discrete_capacity/components/discrete_capacity.pytemoa/extensions/discrete_capacity/core/__init__.pytemoa/extensions/discrete_capacity/core/model.pytemoa/extensions/discrete_capacity/data_manifest.pytemoa/extensions/discrete_capacity/extension.pytemoa/extensions/discrete_capacity/tables.sqltemoa/extensions/framework.pytemoa/extensions/growth_rates/components/growth_capacity.pytests/test_extensions.py
idelder
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE currently based on top of PR #343. This is also different from but compatible with a future unit commitment extension, except that unit commitment divides capacity within a technology into subunits (e.g., gas turbines are each divided into 100 MW sub units) whereas this applies integer capacity units at the technology or tech-group/region summation level, for things like chunky investment/retirement.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation