Add unit commitment extension#350
Open
idelder wants to merge 23 commits into
Open
Conversation
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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (22)
💤 Files with no reviewable changes (3)
WalkthroughIntroduces a configurable extension framework with discrete-capacity, economies-of-scale, growth-rates, and unit-commitment extensions. It consolidates ramp constraints, updates model and loader schemas, revises tutorial assets, and adds extension, sequencing, cost, and integration coverage. ChangesExtension framework and model integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TemoaConfig
participant ExtensionFramework
participant HybridLoader
participant TemoaModel
participant ExtensionHooks
TemoaConfig->>ExtensionFramework: normalize and resolve extension IDs
HybridLoader->>ExtensionFramework: ensure extension tables and build manifest
HybridLoader->>TemoaModel: construct model with enabled extensions
TemoaModel->>ExtensionHooks: register model components
HybridLoader->>ExtensionHooks: load extension manifest items
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
temoa/extensions/monte_carlo/example_builds/scenario_analyzer.py (1)
17-36: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winClose the SQLite connection explicitly.
with Connection(db_resource) as conn:only handles commit/rollback; it does not guaranteeconn.close(). Usecontextlib.closing(...)or atry/finallyto avoid leaking the database handle.🤖 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/monte_carlo/example_builds/scenario_analyzer.py` around lines 17 - 36, Update the database access block around the Connection context manager to explicitly close the SQLite connection, using contextlib.closing or an equivalent try/finally cleanup while preserving the existing query and error behavior.Source: Learnings
🤖 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/unit_commitment.rst`:
- Around line 106-110: The parameter descriptions for min_up_time_hours and
min_down_time_hours incorrectly impose a one-day maximum; update those entries
to describe only their supported non-negative duration requirement, removing the
“must be less than one full day” wording.
In `@scripts/generate_etl_figure.py`:
- Around line 177-187: Update the active-segment label in the ax.text call to
derive its displayed n value from the ACTIVE constant instead of hardcoding
“n=1”, while preserving the existing formatting and highlighting behavior.
In `@temoa/components/operations.py`:
- Around line 195-261: Extract the shared ramping-prefix logic from
_ramp_constraint in temoa/components/operations.py (lines 195-261) into a
reusable helper that returns Constraint.Skip or the computed activity_increase
and rampable-activity base, including parameter lookup, hourly normalization,
ramp_fraction validation, and warning behavior. Update _ramp_constraint to use
it while preserving ramp-up/down inequality construction. In
temoa/extensions/unit_commitment/components/commitment.py (lines 448-557),
replace the duplicated calculations and skip branch with the helper call, then
apply the existing capacity_ramp-specific terms to the returned values.
In `@temoa/data_io/hybrid_loader.py`:
- Around line 659-694: Replace the local TemoaModel() instances in
_load_retired_existing_capacity, _load_lifetime_tech, _load_lifetime_process,
and _load_lifetime_survival_curve with the existing extension-aware self.model
instance. Apply the same replacement to every other model = TemoaModel()
occurrence in this file, reusing self.model when accessing component names.
In `@temoa/extensions/discrete_capacity/components/discrete_capacity.py`:
- Around line 17-26: The set comprehensions are unnecessarily assigned to local
variables before returning. In
temoa/extensions/discrete_capacity/components/discrete_capacity.py lines 17-26,
update limit_discrete_new_capacity_indices to return its comprehension directly;
apply the same change to limit_discrete_capacity_indices at lines 55-64,
preserving both comprehensions’ existing filters and contents.
In `@temoa/extensions/economies_of_scale/components/cost_fixed_eos.py`:
- Around line 44-53: Update the relational checks in the cost_fixed_eos
validation block to wrap each Pyomo component value—cap_lower, cap_upper,
cost_lower, and cost_upper—in value() before applying >= or > comparisons.
Preserve the existing validation conditions and align this evaluation with the
later handling of these variables.
- Around line 69-74: Update the mathematical alignment checks in the cost fixed
EOS validation to evaluate cap_lower, prev_cap_upper, cost_lower, and
prev_cost_upper with Pyomo’s value() before applying abs() and the tolerance
comparisons. Preserve the existing 0.001 threshold and all() structure while
ensuring the comparisons produce Python booleans.
In `@temoa/extensions/economies_of_scale/core/model.py`:
- Around line 17-19: Replace the redundant aliases in
temoa/extensions/economies_of_scale/core/model.py with a grouped import from the
components package for cost_fixed_eos, cost_invest_eos, and cost_variable_eos;
also update temoa/extensions/economies_of_scale/components/cost_invest_eos.py to
import geography and technology directly from temoa.components, preserving all
existing symbol usage.
In `@temoa/extensions/framework.py`:
- Around line 43-46: In the extension ID validation block, update the exception
raised by the type check from ValueError to TypeError while preserving the
existing message and logging behavior.
- Around line 136-139: Replace the explicit loop building populated in the
surrounding function with a list comprehension that retains each table from
spec.owned_tables when _table_has_rows(con, table) is true.
In `@temoa/extensions/growth_rates/components/growth_capacity.py`:
- Around line 40-46: Update both limit_growth_capacity calls in
temoa/extensions/growth_rates/components/growth_capacity.py:40-46 to pass
degrowth=False and degrowth=True as keyword arguments. Apply the same change to
the limit_growth_new_capacity_delta calls in
temoa/extensions/growth_rates/components/growth_new_capacity_delta.py:40-46,
preserving the existing behavior.
- Around line 7-8: Replace the aliased imports with direct imports of geography
and technology from temoa.components in
temoa/extensions/growth_rates/components/growth_capacity.py lines 7-8 and
temoa/extensions/growth_rates/components/growth_new_capacity_delta.py lines 7-8;
preserve all existing module references.
In `@temoa/extensions/monte_carlo/mc_sequencer.py`:
- Around line 153-156: Remove the commented-out kwargs dictionary near the
MCWorker construction in the Monte Carlo sequencer, including its solver_name
and solver_options entries, while leaving the current direct argument passing to
MCWorker unchanged.
In `@temoa/extensions/myopic/myopic_sequencer.py`:
- Around line 316-320: Update the SQL insertion in the sequencer method
containing output_con.execute to use parameterized query arguments instead of
interpolating self.config.scenario and total_cost into the SQL string. Preserve
insertion into output_objective with the existing scenario, total_cost objective
name, and calculated total_cost values.
In `@temoa/extensions/template/data_manifest.py`:
- Around line 19-44: Remove the no-op index_length argument from
build_manifest_items’ LoadItem definition, while leaving the columns,
validation_map, and other loader configuration unchanged.
In `@temoa/extensions/unit_commitment/components/startup.py`:
- Around line 151-160: Update the uc_startup_emissions_rpe call in the
fixed_or_variable_cost comprehension to pass the region as a single-element
collection [r] rather than the string r, ensuring region membership uses exact
element matching instead of substring matching.
In `@temoa/extensions/unit_commitment/core/data_puller.py`:
- Around line 41-51: Restore near-zero filtering in poll_commitment_results and
write_uc_results: only retain commitment rows when online, started, or stopped
exceeds the existing epsilon threshold, matching the filtering pattern used by
poll_startup_input_results and poll_startup_cost_results. Reactivate the
relevant guards and ensure write_uc_results uses its epsilon parameter; remove
the parameter only if filtering is intentionally not required.
In `@temoa/extensions/unit_commitment/core/model.py`:
- Around line 17-18: Replace the aliased module imports for commitment and
startup in model.py with direct from-imports, then update any references in the
module to use the imported symbols without the commitment or startup module
prefixes.
In `@temoa/extensions/unit_commitment/tables.sql`:
- Around line 21-52: Remove the DEFAULT 0.0 clauses from
unit_commitment_startup_emissions.emis_per_cap and
unit_commitment_startup_input.input_per_cap so their declarations no longer
provide values that violate CHECK (... > 0); keep both columns NOT NULL and
preserve the existing positive-value constraints.
In `@tests/testing_data/utopia_v3.sql`:
- Around line 1163-1184: Update the season foreign-key declaration in the
OutputCurtailment table to reference SeasonLabel(season) instead of
TimePeriod(period), matching the schema used by the other season columns in this
file.
---
Outside diff comments:
In `@temoa/extensions/monte_carlo/example_builds/scenario_analyzer.py`:
- Around line 17-36: Update the database access block around the Connection
context manager to explicitly close the SQLite connection, using
contextlib.closing or an equivalent try/finally cleanup while preserving the
existing query and error behavior.
🪄 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: 8840024c-072f-41b8-b623-c3dc45a10db4
⛔ Files ignored due to path filters (4)
docs/source/images/eos_cost_curve.pngis excluded by!**/*.pngdocs/source/images/eos_cost_curve.svgis excluded by!**/*.svgdocs/source/images/etl_cost_curve.pngis excluded by!**/*.pngdocs/source/images/etl_cost_curve.svgis excluded by!**/*.svg
📒 Files selected for processing (116)
.pre-commit-config.yamldocs/source/computational_implementation.rstdocs/source/database_schema.mmddocs/source/extensions.rstdocs/source/extensions/discrete_capacity.rstdocs/source/extensions/eos.rstdocs/source/extensions/growth_rates.rstdocs/source/extensions/unit_commitment.rstdocs/source/index.rstdocs/source/mathematical_formulation.rstdocs/source/param_desc_and_tables.rstpyproject.tomlscripts/generate_etl_figure.pytemoa/__init__.pytemoa/_internal/exchange_tech_cost_ledger.pytemoa/_internal/run_actions.pytemoa/_internal/table_data_puller.pytemoa/_internal/table_writer.pytemoa/_internal/temoa_sequencer.pytemoa/cli.pytemoa/components/capacity.pytemoa/components/commodities.pytemoa/components/costs.pytemoa/components/limits.pytemoa/components/operations.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/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/economies_of_scale/__init__.pytemoa/extensions/economies_of_scale/components/__init__.pytemoa/extensions/economies_of_scale/components/cost_fixed_eos.pytemoa/extensions/economies_of_scale/components/cost_invest_eos.pytemoa/extensions/economies_of_scale/components/cost_variable_eos.pytemoa/extensions/economies_of_scale/core/__init__.pytemoa/extensions/economies_of_scale/core/data_puller.pytemoa/extensions/economies_of_scale/core/model.pytemoa/extensions/economies_of_scale/data_manifest.pytemoa/extensions/economies_of_scale/extension.pytemoa/extensions/economies_of_scale/tables.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/method_of_morris/morris.pytemoa/extensions/method_of_morris/morris_evaluate.pytemoa/extensions/method_of_morris/morris_sequencer.pytemoa/extensions/modeling_to_generate_alternatives/hull.pytemoa/extensions/modeling_to_generate_alternatives/mga_sequencer.pytemoa/extensions/monte_carlo/example_builds/scenario_analyzer.pytemoa/extensions/monte_carlo/example_builds/scenario_maker.pytemoa/extensions/monte_carlo/mc_run.pytemoa/extensions/monte_carlo/mc_sequencer.pytemoa/extensions/myopic/evolution_updater.pytemoa/extensions/myopic/myopic_progress_mapper.pytemoa/extensions/myopic/myopic_sequencer.pytemoa/extensions/single_vector_mga/sv_mga_sequencer.pytemoa/extensions/stochastics/scenario_creator.pytemoa/extensions/stochastics/stochastic_sequencer.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/extensions/unit_commitment/__init__.pytemoa/extensions/unit_commitment/components/__init__.pytemoa/extensions/unit_commitment/components/commitment.pytemoa/extensions/unit_commitment/components/startup.pytemoa/extensions/unit_commitment/core/__init__.pytemoa/extensions/unit_commitment/core/data_puller.pytemoa/extensions/unit_commitment/core/model.pytemoa/extensions/unit_commitment/data_manifest.pytemoa/extensions/unit_commitment/extension.pytemoa/extensions/unit_commitment/tables.sqltemoa/tutorial_assets/config_sample.tomltemoa/tutorial_assets/utopia.sqltemoa/types/__init__.pytemoa/types/dict_types.pytests/conftest.pytests/legacy_test_values.pytests/test_cli.pytests/test_extensions.pytests/test_full_runs.pytests/test_table_writer.pytests/testing_configs/config_myopic_capacities.tomltests/testing_configs/config_test_week.tomltests/testing_data/mediumville.sqltests/testing_data/mediumville_sets.jsontests/testing_data/myopic_capacities.sqltests/testing_data/test_system_sets.jsontests/testing_data/test_week.sqltests/testing_data/utopia_sets.jsontests/testing_data/utopia_v3.sql
💤 Files with no reviewable changes (3)
- tests/testing_data/mediumville.sql
- docs/source/param_desc_and_tables.rst
- docs/source/database_schema.mmd
idelder
force-pushed
the
extension/unit_commitment
branch
from
July 14, 2026 14:51
f85e940 to
9390c47
Compare
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
…P models 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>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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>
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
idelder
force-pushed
the
extension/unit_commitment
branch
2 times, most recently
from
July 14, 2026 14:58
a3a78b1 to
9047c39
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
temoa/components/limits.py (1)
741-757: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocstring math not updated for the new unit-commitment startup emissions term.
The
.. math::block forlimit_emissionstill lists only flow, embodied, and end-of-life emission terms; the newly addedstartup.uc_startup_emissions_rpecontribution (whenunit_commitmentis enabled) isn't reflected in the formula.Also applies to: 798-804
🤖 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/components/limits.py` around lines 741 - 757, Update the limit_emission docstring math block to include the startup.uc_startup_emissions_rpe contribution when unit_commitment is enabled, alongside the existing emission terms and before the comparison to LE. Apply the same formula update to the corresponding math block around the additionally referenced section.temoa/db_schema/temoa_schema_v4.sql (1)
919-938: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore the
output_cost.techforeign key — this table is populated fromTechnology-typed cost keys, and the othertech-bearing output tables still enforcetechnology(tech). Dropping the constraint only weakens integrity checks on persisted results.🤖 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/db_schema/temoa_schema_v4.sql` around lines 919 - 938, Restore the foreign key constraint for output_cost.tech in the output_cost table definition, referencing technology(tech), consistent with the other tech-bearing output tables. Keep the existing columns and constraints unchanged.
♻️ Duplicate comments (2)
temoa/extensions/monte_carlo/mc_sequencer.py (1)
153-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out dead code.
This dictionary is commented out and no longer used, as the arguments are now passed directly to
MCWorker. Removing it improves code readability.♻️ Proposed refactor
- # kwargs = { - # 'solver_name': self.config.solver_name, - # 'solver_options': self.worker_solver_options, - # }🤖 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/monte_carlo/mc_sequencer.py` around lines 153 - 156, Remove the commented-out kwargs dictionary near the MCWorker construction in the Monte Carlo sequencer, including the obsolete solver_name and solver_options entries; leave the current direct argument passing unchanged.temoa/extensions/framework.py (1)
136-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a list comprehension to create the transformed list.
This loop can be simplified for readability and slight performance gains using a list comprehension.
♻️ Proposed refactor
- populated: list[str] = [] - for table in spec.owned_tables: - if _table_has_rows(con, table): - populated.append(table) + populated = [table for table in spec.owned_tables if _table_has_rows(con, table)]🤖 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/framework.py` around lines 136 - 139, Replace the explicit populated-list initialization and loop in the surrounding function with a list comprehension that includes each table from spec.owned_tables only when _table_has_rows(con, table) returns true, preserving the existing order and result.
🤖 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/components/operations.py`:
- Around line 261-283: The ramp-constraint skip/branch wrapper is duplicated
across both implementations. In temoa/components/operations.py lines 261-283,
extract the shared logic around _ramp_constraint so it accepts a domain-specific
rampable-activity callback or precomputed value, then use it for the
capacity-based _rampable_activity path; in
temoa/extensions/unit_commitment/components/commitment.py lines 472-493, replace
the copied None/Constraint.Skip and ramp_up branching with that shared wrapper
while passing the online-units-based _rampable_activity computation.
In `@temoa/components/time.py`:
- Around line 194-198: In the period-length logic, replace the `elif p in
model.time_exist` following the returning branch with an independent `if`,
preserving its existing condition and `return -1` behavior.
- Around line 339-340: Update the adjacent string literals in the relevant error
message so implicit concatenation includes a space between “or” and
“unit_commitment”, preserving the existing message content otherwise.
- Line 413: Update the return expression in the time-of-day calculation to apply
Pyomo’s value() extractor to both model.time_of_day_hours[d] and
model.time_of_day_hours[d_next] before averaging, ensuring the function returns
a native float rather than a Pyomo expression.
In `@temoa/data_io/hybrid_loader.py`:
- Around line 110-116: Initialize self.viable_existing_techs in __init__
alongside viable_existing_rt and viable_existing_rtv, using the same collection
type and empty-value pattern assigned when it is populated during loading.
Remove or avoid relying on its later dynamic creation so early access cannot
raise AttributeError.
In `@temoa/extensions/discrete_capacity/tables.sql`:
- Around line 1-16: Remove the DEFAULT 0 clause from the capacity column in both
limit_discrete_new_capacity and limit_discrete_capacity, keeping capacity
required via NOT NULL so inserts must provide an explicit value and existing
schema constraints remain effective.
In `@temoa/extensions/economies_of_scale/components/cost_fixed_eos.py`:
- Around line 26-81: Extract the shared segment collection and validation
algorithm from cost_fixed_eos.py into a reusable helper, preserving nonnegative
values, strict monotonicity, and 0.001 boundary alignment checks. Update
temoa/extensions/economies_of_scale/components/cost_fixed_eos.py:26-81 to use or
define the helper,
temoa/extensions/economies_of_scale/components/cost_variable_eos.py:26-81 to
call it with cost_variable_eos, cost_variable_eos_segments, and the “activity”
label, and
temoa/extensions/economies_of_scale/components/cost_invest_eos.py:66-110 to call
it with cost_invest_eos, cost_invest_eos_segments, and the “capacity” label;
leave the invest-specific reference-process checks at lines 112-145 unchanged.
In `@temoa/extensions/growth_rates/components/growth_capacity.py`:
- Around line 87-111: Move the gather_group_regions and gather_group_techs calls
for regions and techs from the top of the constraint rule into the nested if p
== model.time_optimize.first() and model.time_exist branch, immediately before
they are used in the existing-capacity filter; leave all other calculations
unchanged.
In `@temoa/extensions/unit_commitment/components/commitment.py`:
- Line 19: The cross-module dependency on _ramp_activity_increase exposes a
private symbol from operations.py. Rename it to a public symbol and update its
definition, imports, and usages in commitment.py and the referenced call sites,
or add a public wrapper and route commitment.py through that API.
- Around line 79-121: Update the hours_back construction in the min up/down
back-slice initialization to iterate over model.uc_unit_capacity.sparse_keys()
rather than the sparse keys of uc_min_down_time_hours and uc_min_up_time_hours.
Include each UC technology’s min up/down hour values, including defaulted zeros,
so uc_backslices entries for hour 0 are created for every capacity-enabled
technology; leave the backtracking logic unchanged.
In `@temoa/extensions/unit_commitment/core/model.py`:
- Around line 89-90: Update the parameter declarations for uc_min_up_time_hours
and uc_min_down_time_hours to use the NonNegativeIntegers domain instead of
Integers, while preserving their existing defaults and indexing.
In `@temoa/extensions/unit_commitment/tables.sql`:
- Around line 31-52: Remove the DEFAULT '' clauses from the emis_comm and
input_comm primary-key columns in unit_commitment_startup_emissions and
unit_commitment_startup_input, leaving both columns as required TEXT values
while preserving their NOT NULL constraints and table keys.
---
Outside diff comments:
In `@temoa/components/limits.py`:
- Around line 741-757: Update the limit_emission docstring math block to include
the startup.uc_startup_emissions_rpe contribution when unit_commitment is
enabled, alongside the existing emission terms and before the comparison to LE.
Apply the same formula update to the corresponding math block around the
additionally referenced section.
In `@temoa/db_schema/temoa_schema_v4.sql`:
- Around line 919-938: Restore the foreign key constraint for output_cost.tech
in the output_cost table definition, referencing technology(tech), consistent
with the other tech-bearing output tables. Keep the existing columns and
constraints unchanged.
---
Duplicate comments:
In `@temoa/extensions/framework.py`:
- Around line 136-139: Replace the explicit populated-list initialization and
loop in the surrounding function with a list comprehension that includes each
table from spec.owned_tables only when _table_has_rows(con, table) returns true,
preserving the existing order and result.
In `@temoa/extensions/monte_carlo/mc_sequencer.py`:
- Around line 153-156: Remove the commented-out kwargs dictionary near the
MCWorker construction in the Monte Carlo sequencer, including the obsolete
solver_name and solver_options entries; leave the current direct argument
passing unchanged.
🪄 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: 160d0b00-cbe1-4d7e-afb3-03e523d4ed23
⛔ Files ignored due to path filters (4)
docs/source/images/eos_cost_curve.pngis excluded by!**/*.pngdocs/source/images/eos_cost_curve.svgis excluded by!**/*.svgdocs/source/images/etl_cost_curve.pngis excluded by!**/*.pngdocs/source/images/etl_cost_curve.svgis excluded by!**/*.svg
📒 Files selected for processing (120)
docs/source/computational_implementation.rstdocs/source/database_schema.mmddocs/source/extensions.rstdocs/source/extensions/discrete_capacity.rstdocs/source/extensions/eos.rstdocs/source/extensions/growth_rates.rstdocs/source/extensions/unit_commitment.rstdocs/source/index.rstdocs/source/mathematical_formulation.rstdocs/source/param_desc_and_tables.rstpyproject.tomlscripts/generate_etl_figure.pytemoa/__init__.pytemoa/_internal/exchange_tech_cost_ledger.pytemoa/_internal/run_actions.pytemoa/_internal/table_data_puller.pytemoa/_internal/table_writer.pytemoa/_internal/temoa_sequencer.pytemoa/cli.pytemoa/components/capacity.pytemoa/components/commodities.pytemoa/components/costs.pytemoa/components/limits.pytemoa/components/operations.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/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/economies_of_scale/__init__.pytemoa/extensions/economies_of_scale/components/__init__.pytemoa/extensions/economies_of_scale/components/cost_fixed_eos.pytemoa/extensions/economies_of_scale/components/cost_invest_eos.pytemoa/extensions/economies_of_scale/components/cost_variable_eos.pytemoa/extensions/economies_of_scale/core/__init__.pytemoa/extensions/economies_of_scale/core/data_puller.pytemoa/extensions/economies_of_scale/core/model.pytemoa/extensions/economies_of_scale/data_manifest.pytemoa/extensions/economies_of_scale/extension.pytemoa/extensions/economies_of_scale/tables.sqltemoa/extensions/framework.pytemoa/extensions/get_comm_tech.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/method_of_morris/morris.pytemoa/extensions/method_of_morris/morris_evaluate.pytemoa/extensions/method_of_morris/morris_sequencer.pytemoa/extensions/modeling_to_generate_alternatives/hull.pytemoa/extensions/modeling_to_generate_alternatives/manager_factory.pytemoa/extensions/modeling_to_generate_alternatives/mga_sequencer.pytemoa/extensions/modeling_to_generate_alternatives/tech_activity_vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/worker.pytemoa/extensions/monte_carlo/example_builds/scenario_analyzer.pytemoa/extensions/monte_carlo/example_builds/scenario_maker.pytemoa/extensions/monte_carlo/mc_run.pytemoa/extensions/monte_carlo/mc_sequencer.pytemoa/extensions/monte_carlo/mc_worker.pytemoa/extensions/myopic/evolution_updater.pytemoa/extensions/myopic/myopic_progress_mapper.pytemoa/extensions/myopic/myopic_sequencer.pytemoa/extensions/single_vector_mga/output_summary.pytemoa/extensions/single_vector_mga/sv_mga_sequencer.pytemoa/extensions/stochastics/scenario_creator.pytemoa/extensions/stochastics/stochastic_config.pytemoa/extensions/stochastics/stochastic_sequencer.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/extensions/unit_commitment/__init__.pytemoa/extensions/unit_commitment/components/__init__.pytemoa/extensions/unit_commitment/components/commitment.pytemoa/extensions/unit_commitment/components/startup.pytemoa/extensions/unit_commitment/core/__init__.pytemoa/extensions/unit_commitment/core/data_puller.pytemoa/extensions/unit_commitment/core/model.pytemoa/extensions/unit_commitment/data_manifest.pytemoa/extensions/unit_commitment/extension.pytemoa/extensions/unit_commitment/tables.sqltemoa/tutorial_assets/config_sample.tomltemoa/tutorial_assets/utopia.sqltemoa/types/__init__.pytemoa/types/dict_types.pytests/conftest.pytests/test_cli.pytests/test_extensions.pytests/test_full_runs.pytests/test_table_writer.pytests/testing_configs/config_myopic_capacities.tomltests/testing_configs/config_test_week.tomltests/testing_data/mediumville_sets.jsontests/testing_data/myopic_capacities.sqltests/testing_data/test_system_sets.jsontests/testing_data/test_week.sqltests/testing_data/utopia_sets.jsontests/testing_data/utopia_v3.sql
💤 Files with no reviewable changes (4)
- temoa/extensions/modeling_to_generate_alternatives/manager_factory.py
- docs/source/database_schema.mmd
- docs/source/param_desc_and_tables.rst
- temoa/extensions/stochastics/stochastic_config.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
temoa/components/limits.py (1)
741-757: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocstring math not updated for the new unit-commitment startup emissions term.
The
.. math::block forlimit_emissionstill lists only flow, embodied, and end-of-life emission terms; the newly addedstartup.uc_startup_emissions_rpecontribution (whenunit_commitmentis enabled) isn't reflected in the formula.Also applies to: 798-804
🤖 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/components/limits.py` around lines 741 - 757, Update the limit_emission docstring math block to include the startup.uc_startup_emissions_rpe contribution when unit_commitment is enabled, alongside the existing emission terms and before the comparison to LE. Apply the same formula update to the corresponding math block around the additionally referenced section.temoa/db_schema/temoa_schema_v4.sql (1)
919-938: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore the
output_cost.techforeign key — this table is populated fromTechnology-typed cost keys, and the othertech-bearing output tables still enforcetechnology(tech). Dropping the constraint only weakens integrity checks on persisted results.🤖 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/db_schema/temoa_schema_v4.sql` around lines 919 - 938, Restore the foreign key constraint for output_cost.tech in the output_cost table definition, referencing technology(tech), consistent with the other tech-bearing output tables. Keep the existing columns and constraints unchanged.
♻️ Duplicate comments (2)
temoa/extensions/monte_carlo/mc_sequencer.py (1)
153-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out dead code.
This dictionary is commented out and no longer used, as the arguments are now passed directly to
MCWorker. Removing it improves code readability.♻️ Proposed refactor
- # kwargs = { - # 'solver_name': self.config.solver_name, - # 'solver_options': self.worker_solver_options, - # }🤖 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/monte_carlo/mc_sequencer.py` around lines 153 - 156, Remove the commented-out kwargs dictionary near the MCWorker construction in the Monte Carlo sequencer, including the obsolete solver_name and solver_options entries; leave the current direct argument passing unchanged.temoa/extensions/framework.py (1)
136-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a list comprehension to create the transformed list.
This loop can be simplified for readability and slight performance gains using a list comprehension.
♻️ Proposed refactor
- populated: list[str] = [] - for table in spec.owned_tables: - if _table_has_rows(con, table): - populated.append(table) + populated = [table for table in spec.owned_tables if _table_has_rows(con, table)]🤖 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/framework.py` around lines 136 - 139, Replace the explicit populated-list initialization and loop in the surrounding function with a list comprehension that includes each table from spec.owned_tables only when _table_has_rows(con, table) returns true, preserving the existing order and result.
🤖 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/components/operations.py`:
- Around line 261-283: The ramp-constraint skip/branch wrapper is duplicated
across both implementations. In temoa/components/operations.py lines 261-283,
extract the shared logic around _ramp_constraint so it accepts a domain-specific
rampable-activity callback or precomputed value, then use it for the
capacity-based _rampable_activity path; in
temoa/extensions/unit_commitment/components/commitment.py lines 472-493, replace
the copied None/Constraint.Skip and ramp_up branching with that shared wrapper
while passing the online-units-based _rampable_activity computation.
In `@temoa/components/time.py`:
- Around line 194-198: In the period-length logic, replace the `elif p in
model.time_exist` following the returning branch with an independent `if`,
preserving its existing condition and `return -1` behavior.
- Around line 339-340: Update the adjacent string literals in the relevant error
message so implicit concatenation includes a space between “or” and
“unit_commitment”, preserving the existing message content otherwise.
- Line 413: Update the return expression in the time-of-day calculation to apply
Pyomo’s value() extractor to both model.time_of_day_hours[d] and
model.time_of_day_hours[d_next] before averaging, ensuring the function returns
a native float rather than a Pyomo expression.
In `@temoa/data_io/hybrid_loader.py`:
- Around line 110-116: Initialize self.viable_existing_techs in __init__
alongside viable_existing_rt and viable_existing_rtv, using the same collection
type and empty-value pattern assigned when it is populated during loading.
Remove or avoid relying on its later dynamic creation so early access cannot
raise AttributeError.
In `@temoa/extensions/discrete_capacity/tables.sql`:
- Around line 1-16: Remove the DEFAULT 0 clause from the capacity column in both
limit_discrete_new_capacity and limit_discrete_capacity, keeping capacity
required via NOT NULL so inserts must provide an explicit value and existing
schema constraints remain effective.
In `@temoa/extensions/economies_of_scale/components/cost_fixed_eos.py`:
- Around line 26-81: Extract the shared segment collection and validation
algorithm from cost_fixed_eos.py into a reusable helper, preserving nonnegative
values, strict monotonicity, and 0.001 boundary alignment checks. Update
temoa/extensions/economies_of_scale/components/cost_fixed_eos.py:26-81 to use or
define the helper,
temoa/extensions/economies_of_scale/components/cost_variable_eos.py:26-81 to
call it with cost_variable_eos, cost_variable_eos_segments, and the “activity”
label, and
temoa/extensions/economies_of_scale/components/cost_invest_eos.py:66-110 to call
it with cost_invest_eos, cost_invest_eos_segments, and the “capacity” label;
leave the invest-specific reference-process checks at lines 112-145 unchanged.
In `@temoa/extensions/growth_rates/components/growth_capacity.py`:
- Around line 87-111: Move the gather_group_regions and gather_group_techs calls
for regions and techs from the top of the constraint rule into the nested if p
== model.time_optimize.first() and model.time_exist branch, immediately before
they are used in the existing-capacity filter; leave all other calculations
unchanged.
In `@temoa/extensions/unit_commitment/components/commitment.py`:
- Line 19: The cross-module dependency on _ramp_activity_increase exposes a
private symbol from operations.py. Rename it to a public symbol and update its
definition, imports, and usages in commitment.py and the referenced call sites,
or add a public wrapper and route commitment.py through that API.
- Around line 79-121: Update the hours_back construction in the min up/down
back-slice initialization to iterate over model.uc_unit_capacity.sparse_keys()
rather than the sparse keys of uc_min_down_time_hours and uc_min_up_time_hours.
Include each UC technology’s min up/down hour values, including defaulted zeros,
so uc_backslices entries for hour 0 are created for every capacity-enabled
technology; leave the backtracking logic unchanged.
In `@temoa/extensions/unit_commitment/core/model.py`:
- Around line 89-90: Update the parameter declarations for uc_min_up_time_hours
and uc_min_down_time_hours to use the NonNegativeIntegers domain instead of
Integers, while preserving their existing defaults and indexing.
In `@temoa/extensions/unit_commitment/tables.sql`:
- Around line 31-52: Remove the DEFAULT '' clauses from the emis_comm and
input_comm primary-key columns in unit_commitment_startup_emissions and
unit_commitment_startup_input, leaving both columns as required TEXT values
while preserving their NOT NULL constraints and table keys.
---
Outside diff comments:
In `@temoa/components/limits.py`:
- Around line 741-757: Update the limit_emission docstring math block to include
the startup.uc_startup_emissions_rpe contribution when unit_commitment is
enabled, alongside the existing emission terms and before the comparison to LE.
Apply the same formula update to the corresponding math block around the
additionally referenced section.
In `@temoa/db_schema/temoa_schema_v4.sql`:
- Around line 919-938: Restore the foreign key constraint for output_cost.tech
in the output_cost table definition, referencing technology(tech), consistent
with the other tech-bearing output tables. Keep the existing columns and
constraints unchanged.
---
Duplicate comments:
In `@temoa/extensions/framework.py`:
- Around line 136-139: Replace the explicit populated-list initialization and
loop in the surrounding function with a list comprehension that includes each
table from spec.owned_tables only when _table_has_rows(con, table) returns true,
preserving the existing order and result.
In `@temoa/extensions/monte_carlo/mc_sequencer.py`:
- Around line 153-156: Remove the commented-out kwargs dictionary near the
MCWorker construction in the Monte Carlo sequencer, including the obsolete
solver_name and solver_options entries; leave the current direct argument
passing unchanged.
🪄 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: 160d0b00-cbe1-4d7e-afb3-03e523d4ed23
⛔ Files ignored due to path filters (4)
docs/source/images/eos_cost_curve.pngis excluded by!**/*.pngdocs/source/images/eos_cost_curve.svgis excluded by!**/*.svgdocs/source/images/etl_cost_curve.pngis excluded by!**/*.pngdocs/source/images/etl_cost_curve.svgis excluded by!**/*.svg
📒 Files selected for processing (120)
docs/source/computational_implementation.rstdocs/source/database_schema.mmddocs/source/extensions.rstdocs/source/extensions/discrete_capacity.rstdocs/source/extensions/eos.rstdocs/source/extensions/growth_rates.rstdocs/source/extensions/unit_commitment.rstdocs/source/index.rstdocs/source/mathematical_formulation.rstdocs/source/param_desc_and_tables.rstpyproject.tomlscripts/generate_etl_figure.pytemoa/__init__.pytemoa/_internal/exchange_tech_cost_ledger.pytemoa/_internal/run_actions.pytemoa/_internal/table_data_puller.pytemoa/_internal/table_writer.pytemoa/_internal/temoa_sequencer.pytemoa/cli.pytemoa/components/capacity.pytemoa/components/commodities.pytemoa/components/costs.pytemoa/components/limits.pytemoa/components/operations.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/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/economies_of_scale/__init__.pytemoa/extensions/economies_of_scale/components/__init__.pytemoa/extensions/economies_of_scale/components/cost_fixed_eos.pytemoa/extensions/economies_of_scale/components/cost_invest_eos.pytemoa/extensions/economies_of_scale/components/cost_variable_eos.pytemoa/extensions/economies_of_scale/core/__init__.pytemoa/extensions/economies_of_scale/core/data_puller.pytemoa/extensions/economies_of_scale/core/model.pytemoa/extensions/economies_of_scale/data_manifest.pytemoa/extensions/economies_of_scale/extension.pytemoa/extensions/economies_of_scale/tables.sqltemoa/extensions/framework.pytemoa/extensions/get_comm_tech.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/method_of_morris/morris.pytemoa/extensions/method_of_morris/morris_evaluate.pytemoa/extensions/method_of_morris/morris_sequencer.pytemoa/extensions/modeling_to_generate_alternatives/hull.pytemoa/extensions/modeling_to_generate_alternatives/manager_factory.pytemoa/extensions/modeling_to_generate_alternatives/mga_sequencer.pytemoa/extensions/modeling_to_generate_alternatives/tech_activity_vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/worker.pytemoa/extensions/monte_carlo/example_builds/scenario_analyzer.pytemoa/extensions/monte_carlo/example_builds/scenario_maker.pytemoa/extensions/monte_carlo/mc_run.pytemoa/extensions/monte_carlo/mc_sequencer.pytemoa/extensions/monte_carlo/mc_worker.pytemoa/extensions/myopic/evolution_updater.pytemoa/extensions/myopic/myopic_progress_mapper.pytemoa/extensions/myopic/myopic_sequencer.pytemoa/extensions/single_vector_mga/output_summary.pytemoa/extensions/single_vector_mga/sv_mga_sequencer.pytemoa/extensions/stochastics/scenario_creator.pytemoa/extensions/stochastics/stochastic_config.pytemoa/extensions/stochastics/stochastic_sequencer.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/extensions/unit_commitment/__init__.pytemoa/extensions/unit_commitment/components/__init__.pytemoa/extensions/unit_commitment/components/commitment.pytemoa/extensions/unit_commitment/components/startup.pytemoa/extensions/unit_commitment/core/__init__.pytemoa/extensions/unit_commitment/core/data_puller.pytemoa/extensions/unit_commitment/core/model.pytemoa/extensions/unit_commitment/data_manifest.pytemoa/extensions/unit_commitment/extension.pytemoa/extensions/unit_commitment/tables.sqltemoa/tutorial_assets/config_sample.tomltemoa/tutorial_assets/utopia.sqltemoa/types/__init__.pytemoa/types/dict_types.pytests/conftest.pytests/test_cli.pytests/test_extensions.pytests/test_full_runs.pytests/test_table_writer.pytests/testing_configs/config_myopic_capacities.tomltests/testing_configs/config_test_week.tomltests/testing_data/mediumville_sets.jsontests/testing_data/myopic_capacities.sqltests/testing_data/test_system_sets.jsontests/testing_data/test_week.sqltests/testing_data/utopia_sets.jsontests/testing_data/utopia_v3.sql
💤 Files with no reviewable changes (4)
- temoa/extensions/modeling_to_generate_alternatives/manager_factory.py
- docs/source/database_schema.mmd
- docs/source/param_desc_and_tables.rst
- temoa/extensions/stochastics/stochastic_config.py
🛑 Comments failed to post (12)
temoa/components/operations.py (1)
261-283: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Ramp-constraint skip/branch wrapper still duplicated across core and UC. The shared-prefix extraction (
_ramp_activity_increase) resolves most of the previously flagged duplication, but the thin wrapper — call the helper, returnConstraint.SkiponNone, then branch onramp_upto buildactivity_increase <= rampableor-activity_increase <= rampable— is copy-pasted in both files.
temoa/components/operations.py#L261-L283: this is the original/simpler version (capacity-based_rampable_activity); a shared wrapper here would need to accept the domain-specific "rampable activity" computation as a callback or pre-computed value.temoa/extensions/unit_commitment/components/commitment.py#L472-L493: mirrors the same skip/branch structure with its own online-units-based_rampable_activity; consolidate by having this call the same shared wrapper, passing its own rampable-activity computation.🧰 Tools
🪛 Ruff (0.15.21)
[warning] 263-263: Boolean-typed positional argument in function definition
(FBT001)
[warning] 282-282: Unnecessary
elseafterreturnstatementRemove unnecessary
else(RET505)
📍 Affects 2 files
temoa/components/operations.py#L261-L283(this comment)temoa/extensions/unit_commitment/components/commitment.py#L472-L493🤖 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/components/operations.py` around lines 261 - 283, The ramp-constraint skip/branch wrapper is duplicated across both implementations. In temoa/components/operations.py lines 261-283, extract the shared logic around _ramp_constraint so it accepts a domain-specific rampable-activity callback or precomputed value, then use it for the capacity-based _rampable_activity path; in temoa/extensions/unit_commitment/components/commitment.py lines 472-493, replace the copied None/Constraint.Skip and ramp_up branching with that shared wrapper while passing the online-units-based _rampable_activity computation.temoa/components/time.py (3)
194-198: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove unnecessary
elifafter areturnstatement.Since the preceding
ifblock ends with areturn, theelifcan be simplified to a simpleifto reduce nested branching.✏️ Proposed fix
if model.time_exist and p == model.time_exist.last(): # Need this for one specific use case (capacity growth constraints) return model.time_future.first() - model.time_exist.last() - elif p in model.time_exist: + if p in model.time_exist: return -1 # Period length is not defined for existing periods except the last📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if model.time_exist and p == model.time_exist.last(): # Need this for one specific use case (capacity growth constraints) return model.time_future.first() - model.time_exist.last() if p in model.time_exist: return -1 # Period length is not defined for existing periods except the last🧰 Tools
🪛 Ruff (0.15.21)
[warning] 197-197: Unnecessary
elifafterreturnstatementRemove unnecessary
elif(RET505)
🤖 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/components/time.py` around lines 194 - 198, In the period-length logic, replace the `elif p in model.time_exist` following the returning branch with an independent `if`, preserving its existing condition and `return -1` behavior.Source: Linters/SAST tools
339-340: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a missing space for string concatenation.
Due to implicit string concatenation, the current strings will evaluate to
... ramp_up/ramp_down orunit_commitment ...with a missing space.✏️ Proposed fix
- 'seasonal storage or inter-season constraints like ramp_up/ramp_down or' - 'unit_commitment are used. Check the config file.' + 'seasonal storage or inter-season constraints like ramp_up/ramp_down or ' + 'unit_commitment are used. Check the config file.'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.'seasonal storage or inter-season constraints like ramp_up/ramp_down or ' 'unit_commitment are used. Check the config file.'🤖 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/components/time.py` around lines 339 - 340, Update the adjacent string literals in the relevant error message so implicit concatenation includes a space between “or” and “unit_commitment”, preserving the existing message content otherwise.
413-413: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use
value()to return a native float instead of a Pyomo expression.
model.time_of_day_hours[...]accesses a Pyomo component data object. Performing arithmetic on it without extracting its value will yield a Pyomo expression tree rather than a nativefloat. When evaluated in boolean contexts (like inequality comparisons) later during unit commitment initialization, Pyomo will raise aPyomoException. Wrapping the parameters invalue()guarantees the function returns a nativefloatas indicated by the type hint.✏️ Proposed fix
- return (model.time_of_day_hours[d] + model.time_of_day_hours[d_next]) / 2 + return (value(model.time_of_day_hours[d]) + value(model.time_of_day_hours[d_next])) / 2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return (value(model.time_of_day_hours[d]) + value(model.time_of_day_hours[d_next])) / 2🤖 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/components/time.py` at line 413, Update the return expression in the time-of-day calculation to apply Pyomo’s value() extractor to both model.time_of_day_hours[d] and model.time_of_day_hours[d_next] before averaging, ensuring the function returns a native float rather than a Pyomo expression.temoa/data_io/hybrid_loader.py (1)
110-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Initialize
viable_existing_techsin__init__.The attribute
self.viable_existing_techsis created dynamically here during data loading. For consistency withviable_existing_rtandviable_existing_rtv, and to follow standard Python best practices (preventing potentialAttributeErrors on early access), please initialize it in__init__.♻️ Proposed refactor
In
__init__:self.viable_existing_rt: set[tuple[Region, Technology]] = set() self.viable_existing_rtv: set[tuple[Region, Technology, Vintage]] = set() + self.viable_existing_techs: set[Technology] = set()Also applies to: 654-658
🤖 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/data_io/hybrid_loader.py` around lines 110 - 116, Initialize self.viable_existing_techs in __init__ alongside viable_existing_rt and viable_existing_rtv, using the same collection type and empty-value pattern assigned when it is populated during loading. Remove or avoid relying on its later dynamic creation so early access cannot raise AttributeError.temoa/extensions/discrete_capacity/tables.sql (1)
1-16: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== tables.sql ==\n' cat -n temoa/extensions/discrete_capacity/tables.sql printf '\n== discrete_capacity.py outline ==\n' ast-grep outline temoa/extensions/discrete_capacity/discrete_capacity.py --view expanded || true printf '\n== search for capacity usage/loaders ==\n' rg -n "limit_discrete_(new_)?capacity|capacity\s*==\s*discrete_cap|v_discrete_new_capacity|v_discrete_capacity|CHECK \(capacity > 0\)|NonNegativeReals|DEFAULT 0" temoa/extensions/discrete_capacity -SRepository: TemoaProject/temoa
Length of output: 6896
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== data manifest / loader outline ==\n' fd -a "data_manifest.py" temoa || true fd -a "*.py" temoa/extensions/discrete_capacity || trueRepository: TemoaProject/temoa
Length of output: 818
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('temoa/extensions/discrete_capacity/tables.sql') print(p.read_text()) PYRepository: TemoaProject/temoa
Length of output: 665
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== data_manifest.py ==\n' cat -n temoa/extensions/discrete_capacity/data_manifest.py printf '\n== core/model.py ==\n' cat -n temoa/extensions/discrete_capacity/core/model.py printf '\n== extension.py ==\n' cat -n temoa/extensions/discrete_capacity/extension.pyRepository: TemoaProject/temoa
Length of output: 4815
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== search for discrete_capacity data files ==\n' fd -a --type f 'temoa/extensions/discrete_capacity' | sed 's#^\./##' | sort printf '\n== search for table schemas / csv headers referencing capacity ==\n' rg -n "limit_discrete_(new_)?capacity|capacity" temoa/extensions/discrete_capacity -g '*.csv' -g '*.sql' -g '*.py' -SRepository: TemoaProject/temoa
Length of output: 645
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== loader_manifest.py outline ==\n' ast-grep outline temoa/data_io/loader_manifest.py --view expanded || true printf '\n== loader_manifest.py relevant lines ==\n' rg -n "class LoadItem|columns|validator_name|validation_map|INSERT|executemany|DEFAULT|NULL|missing" temoa/data_io/loader_manifest.py temoa/data_io -S printf '\n== discrete capacity component loader/search ==\n' rg -n "LoadItem|is_table_required|columns=\['region', 'tech_or_group', 'capacity'\]" temoa/extensions/discrete_capacity temoa/data_io -SRepository: TemoaProject/temoa
Length of output: 33579
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== data_io/loader_manifest.py ==\n' cat -n temoa/data_io/loader_manifest.py printf '\n== data_io search for table insertion logic ==\n' rg -n "LoadItem|INSERT INTO|insert into|to_sql|sqlite|default 0|missing" temoa/data_io -SRepository: TemoaProject/temoa
Length of output: 11853
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== hybrid_loader optional-table handling ==\n' sed -n '300,380p' temoa/data_io/hybrid_loader.py printf '\n== search for direct inserts / seed data for discrete_capacity ==\n' rg -n "INSERT INTO limit_discrete_(new_)?capacity|limit_discrete_(new_)?capacity.*VALUES|fallback_data=.*limit_discrete_(new_)?capacity|discrete_capacity" temoa -S printf '\n== discrete_capacity docs/files ==\n' fd -a . temoa/extensions/discrete_capacity -t f | sortRepository: TemoaProject/temoa
Length of output: 7793
Avoid
DEFAULT 0oncapacity. If a row is inserted without an explicit value, SQLite stores0, and the discrete-capacity constraint collapses to== 0for that region/tech-or-group.CHECK (capacity > 0)would fail fast instead of silently disabling the limit.🤖 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, Remove the DEFAULT 0 clause from the capacity column in both limit_discrete_new_capacity and limit_discrete_capacity, keeping capacity required via NOT NULL so inserts must provide an explicit value and existing schema constraints remain effective.temoa/extensions/economies_of_scale/components/cost_fixed_eos.py (1)
26-81: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Extract the shared EOS segment-validation logic into one helper. All three EOS cost-curve modules implement the same "collect segments, then validate nonnegativity, strict monotonicity, and 0.001 boundary alignment" algorithm almost verbatim, differing only in which param/table and index tuple they read.
temoa/extensions/economies_of_scale/components/cost_fixed_eos.py#L26-L81: keep as the reference implementation, or convert to a thin wrapper around the extracted helper.temoa/extensions/economies_of_scale/components/cost_variable_eos.py#L26-L81: replace this near-identical block with a call to the same shared helper, passingcost_variable_eos/cost_variable_eos_segmentsand an "activity" label for error messages.temoa/extensions/economies_of_scale/components/cost_invest_eos.py#L66-L110: replace the segment-validation portion (excluding the invest-specific reference-process checks at lines 112-145) with a call to the same shared helper, passingcost_invest_eos/cost_invest_eos_segmentsand a "capacity" label.📍 Affects 3 files
temoa/extensions/economies_of_scale/components/cost_fixed_eos.py#L26-L81(this comment)temoa/extensions/economies_of_scale/components/cost_variable_eos.py#L26-L81temoa/extensions/economies_of_scale/components/cost_invest_eos.py#L66-L110🤖 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/economies_of_scale/components/cost_fixed_eos.py` around lines 26 - 81, Extract the shared segment collection and validation algorithm from cost_fixed_eos.py into a reusable helper, preserving nonnegative values, strict monotonicity, and 0.001 boundary alignment checks. Update temoa/extensions/economies_of_scale/components/cost_fixed_eos.py:26-81 to use or define the helper, temoa/extensions/economies_of_scale/components/cost_variable_eos.py:26-81 to call it with cost_variable_eos, cost_variable_eos_segments, and the “activity” label, and temoa/extensions/economies_of_scale/components/cost_invest_eos.py:66-110 to call it with cost_invest_eos, cost_invest_eos_segments, and the “capacity” label; leave the invest-specific reference-process checks at lines 112-145 unchanged.temoa/extensions/growth_rates/components/growth_capacity.py (1)
87-111: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Compute
regions/techslazily, only when needed.
regionsandtechsare only used inside theif p == model.time_optimize.first(): if model.time_exist:branch (Lines 101-111), but they're computed unconditionally at Lines 87-88 for every call of this constraint rule. The siblinggrowth_new_capacity.pycomputes the equivalent values lazily inside that same branch. Move the computation inside the conditional to avoid the wasted work on every non-first-period call.♻️ Proposed fix
- regions = geography.gather_group_regions(model, r) - techs = technology.gather_group_techs(model, t) - growth = model.limit_degrowth_capacity if degrowth else model.limit_growth_capacity rate = 1 + value(growth[r, t, op][0]) seed = value(growth[r, t, op][1]) cap = quicksum( model.v_capacity_available_by_period_and_tech[_r, p, _t] for _r, _t in capacity.gather_group_active_processes(model, r, p, t) ) capacity_prev = 0.0 if p == model.time_optimize.first(): if model.time_exist: + regions = geography.gather_group_regions(model, r) + techs = technology.gather_group_techs(model, t) p_prev = model.time_exist.last()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.growth = model.limit_degrowth_capacity if degrowth else model.limit_growth_capacity rate = 1 + value(growth[r, t, op][0]) seed = value(growth[r, t, op][1]) cap = quicksum( model.v_capacity_available_by_period_and_tech[_r, p, _t] for _r, _t in capacity.gather_group_active_processes(model, r, p, t) ) capacity_prev = 0.0 if p == model.time_optimize.first(): if model.time_exist: regions = geography.gather_group_regions(model, r) techs = technology.gather_group_techs(model, t) p_prev = model.time_exist.last() capacity_prev = quicksum( get_adjusted_existing_capacity(model, _r, _t, _v) * value(model.process_life_frac[_r, p_prev, _t, _v]) for _r, _t, _v in model.existing_capacity.sparse_keys() if _r in regions and _t in techs and _v + value(model.lifetime_process[_r, _t, _v]) > p_prev )🤖 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/growth_rates/components/growth_capacity.py` around lines 87 - 111, Move the gather_group_regions and gather_group_techs calls for regions and techs from the top of the constraint rule into the nested if p == model.time_optimize.first() and model.time_exist branch, immediately before they are used in the existing-capacity filter; leave all other calculations unchanged.temoa/extensions/unit_commitment/components/commitment.py (2)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Imports a leading-underscore ("private") symbol across modules.
_ramp_activity_increaseis defined with a leading underscore intemoa/components/operations.py, signaling module-private, but is imported and used directly here. Consider dropping the underscore (or introducing a small public wrapper) now that it's a cross-module dependency, so its "private" naming doesn't mislead future maintainers ofoperations.pyinto refactoring/removing it without checkingcommitment.py.Also applies to: 448-493
🤖 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/unit_commitment/components/commitment.py` at line 19, The cross-module dependency on _ramp_activity_increase exposes a private symbol from operations.py. Rename it to a public symbol and update its definition, imports, and usages in commitment.py and the referenced call sites, or add a public wrapper and route commitment.py through that API.
79-121: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Files ==\n' git ls-files 'temoa/extensions/unit_commitment/components/commitment.py' 'temoa/components/operations.py' 'temoa/extensions/unit_commitment/model.py' 2>/dev/null || true printf '\n== commitment.py around hours_back and constraints ==\n' sed -n '70,150p' temoa/extensions/unit_commitment/components/commitment.py printf '\n---\n' sed -n '380,470p' temoa/extensions/unit_commitment/components/commitment.py printf '\n== model.py unit commitment param definitions ==\n' sed -n '70,110p' temoa/extensions/unit_commitment/model.py printf '\n== operations.py relevant helper/constraint ==\n' sed -n '1,120p' temoa/components/operations.py printf '\n---\n' sed -n '430,520p' temoa/components/operations.pyRepository: TemoaProject/temoa
Length of output: 7119
🌐 Web query:
Pyomo Param sparse_keys default values semantics site:pyomo.readthedocs.io sparse_keys default💡 Result:
In Pyomo, the
sparse_keys()method of aParamobject is designed to return only the indices for which values have been explicitly defined, effectively ignoring any default value that might be set for the parameter [1][2]. The internal implementation ofsparse_keys()achieves this by temporarily suppressing the default value of theParamcomponent [1][2]. Specifically: 1. It saves the current default value (which could be a constant, a rule/function, orParam.NoValue) [1][2]. 2. It sets the component's default value toParam.NoValue[1][2]. 3. It calls the standardkeys()method, which, without the default value, only iterates over the keys present in the_datadictionary (the explicitly defined entries) [1][2]. 4. It restores the original default value in afinallyblock to ensure the component's state remains unchanged [1][2]. This ensures thatsparse_keys()provides a "sparse" view of the parameter, distinguishing between entries that were explicitly assigned and those that would simply inherit the default value if accessed [1][2].Citations:
- 1: https://pyomo.readthedocs.io/en/stable/_modules/pyomo/core/base/param.html
- 2: https://pyomo.readthedocs.io/en/latest/%5Fmodules/pyomo/core/base/param.html
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import pathlib, re p = pathlib.Path('temoa/extensions/unit_commitment/components/commitment.py') text = p.read_text() for pat in [ r'uc_backslices', r'uc_min_up_time_hours', r'uc_min_down_time_hours', r'uc_indices_rpsdtv', ]: print(f'\n## {pat}') for m in re.finditer(pat, text): start = max(0, text.rfind('\n', 0, m.start()-80)) end = text.find('\n', m.end()+160) print(text[start+1:end+1]) break PYRepository: TemoaProject/temoa
Length of output: 1608
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '== parameter and backslices declarations ==\n' rg -n "uc_min_(up|down)_time_hours|uc_backslices|uc_indices_rpsdtv|default *= *0" temoa -g '*.py' printf '\n== surrounding declaration context ==\n' python3 - <<'PY' from pathlib import Path paths = [ Path('temoa/extensions/unit_commitment/components/commitment.py'), Path('temoa/extensions/unit_commitment/model.py'), Path('temoa/extensions/unit_commitment/__init__.py'), ] for path in paths: if path.exists(): print(f'\n### {path} ({sum(1 for _ in path.open())} lines)') text = path.read_text().splitlines() for needle in ['uc_min_up_time_hours', 'uc_min_down_time_hours', 'uc_backslices', 'uc_indices_rpsdtv']: for i, line in enumerate(text, 1): if needle in line: start = max(1, i-5) end = min(len(text), i+8) for j in range(start, end+1): print(f'{j:4d}: {text[j-1]}') print('---') break PYRepository: TemoaProject/temoa
Length of output: 8806
Populate
hours_backfrom the UC capacity universe, not the sparse min-time params
Param.sparse_keys()skips entries that fall back todefault=0, so any UC tech that leavesuc_min_up_time_hoursoruc_min_down_time_hoursunset will later hitmodel.uc_backslices[s, d, 0]in the min-up/min-down constraints and fail model construction. Build the lookback set frommodel.uc_unit_capacity.sparse_keys()instead so defaulted zeros are included.Proposed fix
- hours_back = { - int(value(model.uc_min_down_time_hours[r, t])) - for (r, t) in model.uc_min_down_time_hours.sparse_keys() - } - hours_back |= { - int(value(model.uc_min_up_time_hours[r, t])) - for (r, t) in model.uc_min_up_time_hours.sparse_keys() - } + hours_back = { + int(value(model.uc_min_down_time_hours[r, t])) + for (r, t) in model.uc_unit_capacity.sparse_keys() + } + hours_back |= { + int(value(model.uc_min_up_time_hours[r, t])) + for (r, t) in model.uc_unit_capacity.sparse_keys() + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.# Initialise the time slice back-check lists for min up/down time constraints # Tells us which previous time slices' startups/shutdowns are relevant epsilon = 1e-6 # To buffer float/integer interactions # We only care about how many hours back we're looking, so why redo work? hours_back = { int(value(model.uc_min_down_time_hours[r, t])) for (r, t) in model.uc_unit_capacity.sparse_keys() } hours_back |= { int(value(model.uc_min_up_time_hours[r, t])) for (r, t) in model.uc_unit_capacity.sparse_keys() } # Invert the time next sequence to start... time_prev = {model.time_next[s, d]: (s, d) for s, d in model.time_next} # Then back down the chain until the next timeslice back is irrelevant for hb in hours_back: for s, d in time_prev: model.uc_backslices[s, d, hb] = set() _s, _d = s, d _hours = float(hb) i = 0 # Some loop safety while i + 1 < len(model.time_next): s_prev, d_prev = time_prev[_s, _d] if s_prev == _s and d_prev == _d: msg = ( 'When finding relevant past time slices for min up/downtime ' 'constraints, looped back to the same time slice. This would ' 'likely cause infeasibility and is not supported. If the ' 'min up/downtime is longer than a season, try instead ' 'flagging the technology as production baseload "pb" and ' 'setting min up/down times to zero to skip the constraints. ' ) logger.error(msg) raise ValueError(msg) elapsed = tod_elapsed_hours(model, d_prev, _d) _hours -= elapsed if _hours < -epsilon: break # too far back (with some rounding buffer) so ignore i += 1 model.uc_backslices[s, d, hb].add((s_prev, d_prev)) _s, _d = s_prev, d_prev🤖 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/unit_commitment/components/commitment.py` around lines 79 - 121, Update the hours_back construction in the min up/down back-slice initialization to iterate over model.uc_unit_capacity.sparse_keys() rather than the sparse keys of uc_min_down_time_hours and uc_min_up_time_hours. Include each UC technology’s min up/down hour values, including defaulted zeros, so uc_backslices entries for hour 0 are created for every capacity-enabled technology; leave the backtracking logic unchanged.temoa/extensions/unit_commitment/core/model.py (1)
89-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the relevant file and locate the initialization routine. ast-grep outline temoa/extensions/unit_commitment/core/model.py --view expanded || true rg -n "uc_min_up_time_hours|uc_min_down_time_hours|initialize_unit_commitment|NonNegativeIntegers|Integers" temoa/extensions/unit_commitment/core/model.py temoa/extensions/unit_commitment/core -S # Show the relevant slice of the model file. sed -n '1,220p' temoa/extensions/unit_commitment/core/model.pyRepository: TemoaProject/temoa
Length of output: 9897
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the initialization logic and any parameter validation around unit commitment. ast-grep outline temoa/extensions/unit_commitment/components/commitment.py --view expanded || true rg -n "min_up_time_hours|min_down_time_hours|backslice|epsilon|validate|domain=NonNegativeIntegers|NonNegativeIntegers" temoa/extensions/unit_commitment/components/commitment.py temoa/extensions/unit_commitment/components -S # Read the relevant sections of the commitment component. sed -n '1,260p' temoa/extensions/unit_commitment/components/commitment.pyRepository: TemoaProject/temoa
Length of output: 14751
Tighten min up/down time params to non-negative integers. Negative values are accepted today and just produce empty back-slice sets in
initialize_unit_commitment, which hides bad input data. Switching both params toNonNegativeIntegerswould fail fast on invalid hours.🤖 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/unit_commitment/core/model.py` around lines 89 - 90, Update the parameter declarations for uc_min_up_time_hours and uc_min_down_time_hours to use the NonNegativeIntegers domain instead of Integers, while preserving their existing defaults and indexing.temoa/extensions/unit_commitment/tables.sql (1)
31-52: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n "DEFAULT ''" temoa/db_schema/temoa_schema_v4.sqlRepository: TemoaProject/temoa
Length of output: 156
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- all DEFAULT empty-string occurrences ---' rg -n "DEFAULT ''" temoa -g '*.sql' -g '*.sqlite' -g '*.py' -g '*.ts' -g '*.js' || true echo echo '--- relevant table definitions ---' sed -n '1,120p' temoa/extensions/unit_commitment/tables.sql echo echo '--- any references to the startup tables ---' rg -n "unit_commitment_startup_(emissions|input)|emis_comm|input_comm" temoa -g '*.sql' -g '*.py' -g '*.ts' -g '*.js' -g '*.md' || trueRepository: TemoaProject/temoa
Length of output: 25911
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- unit commitment data manifest ---' sed -n '1,140p' temoa/extensions/unit_commitment/data_manifest.py echo echo '--- table writer sections touching these tables ---' rg -n -A4 -B4 "unit_commitment_startup_(emissions|input)|emis_comm|input_comm" temoa/_internal/table_writer.py temoa/data_io/component_manifest.py temoa/extensions/unit_commitment -g '*.py' -g '*.sql' || true echo echo '--- any schema docs or migration notes around startup tables ---' rg -n "unit_commitment_startup_(emissions|input)|DEFAULT ''|emis_comm.*DEFAULT|input_comm.*DEFAULT" temoa/db_schema temoa/extensions/unit_commitment temoa -g '*.md' -g '*.sql' -g '*.py' || trueRepository: TemoaProject/temoa
Length of output: 21084
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- temoa_schema_v4.sql around unit commitment startup tables ---' sed -n '288,323p' temoa/db_schema/temoa_schema_v4.sql echo echo '--- temoa_schema_v3.sql around unit commitment startup tables ---' sed -n '585,615p' temoa/db_schema/temoa_schema_v3.sql echo echo '--- data manifest for the startup tables ---' sed -n '55,90p' temoa/extensions/unit_commitment/data_manifest.pyRepository: TemoaProject/temoa
Length of output: 3058
Remove the empty-string defaults on the PK columns.
emis_commandinput_commshould be required values here; otherwise an omitted field can persist as''and create an ambiguous primary-key row.🤖 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/unit_commitment/tables.sql` around lines 31 - 52, Remove the DEFAULT '' clauses from the emis_comm and input_comm primary-key columns in unit_commitment_startup_emissions and unit_commitment_startup_input, leaving both columns as required TEXT values while preserving their NOT NULL constraints and table keys.
idelder
force-pushed
the
extension/unit_commitment
branch
from
July 14, 2026 15:21
d6cc97b to
a5b0d1d
Compare
idelder
force-pushed
the
extension/unit_commitment
branch
from
July 14, 2026 19:21
3fe93ef to
c617d2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
temoa/extensions/get_comm_tech.py (1)
23-41: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winEnsure the database connection is reliably closed.
If an exception occurs during the database operations,
con.close()will be bypassed and the connection will leak. Based on learnings, when working with SQLite in this codebase, preferwith contextlib.closing(sqlite3.connect(...)) as con:(or atry/finallyblock) to ensure the connection is explicitly closed on exit.♻️ Proposed refactor
- con = sqlite3.connect(inp_f) - cur = con.cursor() # a database cursor is a control structure that enables traversal over - # the records in a database - con.text_factory = str # this ensures data is explored with the correct UTF-8 encoding + import contextlib - print(inp_f) - cur.execute('SELECT DISTINCT scenario FROM output_flow_out') - x = [] - for row in cur: - x.append(row[0]) - for y in x: - cur.execute('SELECT DISTINCT period FROM output_flow_out WHERE scenario = ?', (y,)) - periods_list[y] = [] - for per in cur: - z = per[0] - periods_list[y].append(z) + with contextlib.closing(sqlite3.connect(inp_f)) as con: + cur = con.cursor() # a database cursor is a control structure that enables traversal over + # the records in a database + con.text_factory = str # this ensures data is explored with the correct UTF-8 encoding - cur.close() - con.close() + print(inp_f) + cur.execute('SELECT DISTINCT scenario FROM output_flow_out') + x = [] + for row in cur: + x.append(row[0]) + for y in x: + cur.execute('SELECT DISTINCT period FROM output_flow_out WHERE scenario = ?', (y,)) + periods_list[y] = [] + for per in cur: + z = per[0] + periods_list[y].append(z) + + cur.close()🤖 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/get_comm_tech.py` around lines 23 - 41, Update the database handling around sqlite3.connect in the surrounding function to use contextlib.closing (or an equivalent try/finally), ensuring con.close() executes when any query or iteration raises; keep the existing cursor cleanup and query behavior unchanged.Source: Learnings
temoa/extensions/unit_commitment/core/data_puller.py (1)
42-50: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winExtract the numeric value from the
uc_unit_capacityparameter.
model.uc_unit_capacityis a Pyomo parameter. By omittingvalue(),unit_capis assigned a Pyomo component object rather than a native float. Multiplying this object by the variable values (value(model.v_uc_online...)) results in PyomoExpressionobjects. When theif online > 0 or started > 0 or stopped > 0:condition subsequently attempts to evaluate the truth value of these expressions, Pyomo will raise aPyomoException("Cannot convert non-constant Pyomo expression to bool") and crash the data puller.Wrap
model.uc_unit_capacity[r, t]withvalue()to ensureonline,started, andstoppedare evaluated safely as native floats.🐛 Proposed fix
for r, p, s, d, t, v in model.uc_indices_rpsdtv: - unit_cap = model.uc_unit_capacity[r, t] + unit_cap = value(model.uc_unit_capacity[r, t]) online = unit_cap * value(model.v_uc_online[r, p, s, d, t, v])🤖 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/unit_commitment/core/data_puller.py` around lines 42 - 50, Update the unit_cap assignment in the loop over model.uc_indices_rpsdtv to wrap model.uc_unit_capacity[r, t] with value(), ensuring the subsequent online, started, stopped calculations and conditional use native numeric values.
🤖 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/components/limits.py`:
- Around line 801-803: Rename the generator expression’s region loop variable in
the emissions calculation within limit_emission_constraint from r to reg, and
update the corresponding uc_startup_emissions_rpe argument. Leave the function
parameter r and all other logic unchanged.
In `@temoa/extensions/economies_of_scale/components/cost_invest_eos.py`:
- Around line 400-412: Update the capacity aggregation in the segment lookup
block to use Python’s built-in sum() over the value-extracted existing_capacity
entries instead of quicksum(). In the loop over model.cost_invest_eos_segments,
replace continue after setting prev_cum_cost and in_bounds with break so
iteration stops once the matching segment is found.
---
Outside diff comments:
In `@temoa/extensions/get_comm_tech.py`:
- Around line 23-41: Update the database handling around sqlite3.connect in the
surrounding function to use contextlib.closing (or an equivalent try/finally),
ensuring con.close() executes when any query or iteration raises; keep the
existing cursor cleanup and query behavior unchanged.
In `@temoa/extensions/unit_commitment/core/data_puller.py`:
- Around line 42-50: Update the unit_cap assignment in the loop over
model.uc_indices_rpsdtv to wrap model.uc_unit_capacity[r, t] with value(),
ensuring the subsequent online, started, stopped calculations and conditional
use native numeric values.
🪄 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: e9b49697-4f9b-4f62-83dc-92bd060c0bef
📒 Files selected for processing (21)
docs/source/extensions/unit_commitment.rsttemoa/components/limits.pytemoa/data_io/hybrid_loader.pytemoa/extensions/discrete_capacity/core/model.pytemoa/extensions/economies_of_scale/components/cost_invest_eos.pytemoa/extensions/economies_of_scale/core/model.pytemoa/extensions/framework.pytemoa/extensions/get_comm_tech.pytemoa/extensions/method_of_morris/morris_evaluate.pytemoa/extensions/modeling_to_generate_alternatives/manager_factory.pytemoa/extensions/modeling_to_generate_alternatives/tech_activity_vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/worker.pytemoa/extensions/monte_carlo/mc_worker.pytemoa/extensions/single_vector_mga/output_summary.pytemoa/extensions/stochastics/stochastic_config.pytemoa/extensions/template/core/model.pytemoa/extensions/unit_commitment/components/startup.pytemoa/extensions/unit_commitment/core/data_puller.pytemoa/extensions/unit_commitment/core/model.pytests/test_full_runs.py
💤 Files with no reviewable changes (3)
- temoa/extensions/stochastics/stochastic_config.py
- temoa/extensions/modeling_to_generate_alternatives/manager_factory.py
- temoa/extensions/discrete_capacity/core/model.py
idelder
force-pushed
the
extension/unit_commitment
branch
from
July 14, 2026 19:52
5a33281 to
52e847e
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
temoa/extensions/get_comm_tech.py (1)
23-41: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEnsure the database connection is explicitly closed on exception.
If an exception occurs during the cursor execution or iteration, the connection will not be closed, leading to a resource leak. Based on learnings,
sqlite3connections in this codebase should be managed withcontextlib.closingor atry/finallyblock to ensure they are explicitly closed on exit.🛡️ Proposed fix
- con = sqlite3.connect(inp_f) - cur = con.cursor() # a database cursor is a control structure that enables traversal over - # the records in a database - con.text_factory = str # this ensures data is explored with the correct UTF-8 encoding - - print(inp_f) - cur.execute('SELECT DISTINCT scenario FROM output_flow_out') - x = [] - for row in cur: - x.append(row[0]) - for y in x: - cur.execute('SELECT DISTINCT period FROM output_flow_out WHERE scenario = ?', (y,)) - periods_list[y] = [] - for per in cur: - z = per[0] - periods_list[y].append(z) - - cur.close() - con.close() + con = sqlite3.connect(inp_f) + try: + cur = con.cursor() # a database cursor is a control structure that enables traversal over + # the records in a database + con.text_factory = str # this ensures data is explored with the correct UTF-8 encoding + + print(inp_f) + cur.execute('SELECT DISTINCT scenario FROM output_flow_out') + x = [] + for row in cur: + x.append(row[0]) + for y in x: + cur.execute('SELECT DISTINCT period FROM output_flow_out WHERE scenario = ?', (y,)) + periods_list[y] = [] + for per in cur: + z = per[0] + periods_list[y].append(z) + + cur.close() + finally: + con.close()🤖 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/get_comm_tech.py` around lines 23 - 41, Update the database access flow around the connection and cursor creation in get_comm_tech to guarantee both resources are closed when query execution or iteration raises an exception. Use contextlib.closing or an equivalent try/finally structure, while preserving the existing query and periods_list population behavior.Source: Learnings
temoa/data_io/hybrid_loader.py (2)
708-710: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
member_tuplesfor consistency withrtvviability sets.For consistency with
self.viable_rtv.member_tuplesused in_load_lifetime_processand_load_lifetime_survival_curve, prefer usingmember_tuplesforself.viable_rtto ensure explicit handling of tuples.♻️ Proposed refactor
rt_getter = itemgetter(0, 1) if self.viable_rt: - valid_rt = self.viable_rt.members | self.viable_existing_rt + valid_rt = self.viable_rt.member_tuples | self.viable_existing_rt rows_to_load = [item for item in rows_to_load if rt_getter(item) in valid_rt]🤖 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/data_io/hybrid_loader.py` around lines 708 - 710, Update the viable_rt filtering block to use self.viable_rt.member_tuples when constructing valid_rt, while retaining self.viable_existing_rt and the existing rt_getter-based filtering. This aligns the logic with the member_tuples handling in _load_lifetime_process and _load_lifetime_survival_curve.
113-115: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winInitialize
self.viable_existing_techs.
self.viable_existing_techsis conditionally assigned in_load_existing_capacity, but it is not initialized in__init__. Ifrows_to_loadis empty, this attribute will remain undeclared, potentially causing anAttributeErrorif accessed elsewhere.🛡️ Proposed fix
self.data: dict[str, object] | None = None + self.viable_existing_techs = set() self.viable_existing_rt: set[tuple[Region, Technology]] = set() self.viable_existing_rtv: set[tuple[Region, Technology, Vintage]] = set()🤖 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/data_io/hybrid_loader.py` around lines 113 - 115, Initialize self.viable_existing_techs in __init__, alongside viable_existing_rt and viable_existing_rtv, using the collection type expected by _load_existing_capacity. Preserve the existing conditional assignment and ensure the attribute exists even when rows_to_load is empty.
🤖 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.
Outside diff comments:
In `@temoa/data_io/hybrid_loader.py`:
- Around line 708-710: Update the viable_rt filtering block to use
self.viable_rt.member_tuples when constructing valid_rt, while retaining
self.viable_existing_rt and the existing rt_getter-based filtering. This aligns
the logic with the member_tuples handling in _load_lifetime_process and
_load_lifetime_survival_curve.
- Around line 113-115: Initialize self.viable_existing_techs in __init__,
alongside viable_existing_rt and viable_existing_rtv, using the collection type
expected by _load_existing_capacity. Preserve the existing conditional
assignment and ensure the attribute exists even when rows_to_load is empty.
In `@temoa/extensions/get_comm_tech.py`:
- Around line 23-41: Update the database access flow around the connection and
cursor creation in get_comm_tech to guarantee both resources are closed when
query execution or iteration raises an exception. Use contextlib.closing or an
equivalent try/finally structure, while preserving the existing query and
periods_list population behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b435d5c0-9ce4-4dd4-bb6c-744e731057ec
📒 Files selected for processing (21)
docs/source/extensions/unit_commitment.rsttemoa/components/limits.pytemoa/data_io/hybrid_loader.pytemoa/extensions/discrete_capacity/core/model.pytemoa/extensions/economies_of_scale/components/cost_invest_eos.pytemoa/extensions/economies_of_scale/core/model.pytemoa/extensions/framework.pytemoa/extensions/get_comm_tech.pytemoa/extensions/method_of_morris/morris_evaluate.pytemoa/extensions/modeling_to_generate_alternatives/manager_factory.pytemoa/extensions/modeling_to_generate_alternatives/tech_activity_vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/vector_manager.pytemoa/extensions/modeling_to_generate_alternatives/worker.pytemoa/extensions/monte_carlo/mc_worker.pytemoa/extensions/single_vector_mga/output_summary.pytemoa/extensions/stochastics/stochastic_config.pytemoa/extensions/template/core/model.pytemoa/extensions/unit_commitment/components/startup.pytemoa/extensions/unit_commitment/core/data_puller.pytemoa/extensions/unit_commitment/core/model.pytests/test_full_runs.py
💤 Files with no reviewable changes (3)
- temoa/extensions/stochastics/stochastic_config.py
- temoa/extensions/modeling_to_generate_alternatives/manager_factory.py
- temoa/extensions/discrete_capacity/core/model.py
Signed-off-by: Davey Elder <iandavidelder@gmail.com>
idelder
force-pushed
the
extension/unit_commitment
branch
from
July 16, 2026 17:25
b36a6d9 to
be883c7
Compare
for more information, see https://pre-commit.ci
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.
Adds integer and linearised unit commitment as an optional extension.
Docs:
Unit Commitment.pdf
Summary by CodeRabbit
Summary
extensionsconfiguration option.