bug: PS ramp_limits and time_limits #180
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #180 +/- ##
==========================================
+ Coverage 83.28% 83.31% +0.03%
==========================================
Files 42 44 +2
Lines 4504 4614 +110
==========================================
+ Hits 3751 3844 +93
- Misses 753 770 +17
🚀 New features to boost your workflow:
|
|
@pesap - Ready for your review now. Seem to have addressed |
There was a problem hiding this comment.
Pull Request Overview
This PR revamps PCM defaults integration by introducing explicit ramp and time limit structures, extends generator models with these new fields, and updates time series exporter/parser logic to use unified name and initial_timestamp attributes.
- Overhauled
pcm_defaultsplugin to consume newramp_limitsandtime_limitsJSON schema. - Added
ramp_limitsandtime_limitstoGeneratorandHydroPumpedStoragemodels with default values. - Updated parser and exporter functions (ReEDS, PLEXOS, Sienna) and corresponding tests to use
nameandinitial_timestampfor time series metadata.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_sienna_exporter.py | Adapt test to positional-only signature of create_timeseries_pointers |
| tests/test_plugin_pcm_defaults.py | Validate new PCM defaults JSON structure (ramp_limits, time_limits) |
| tests/test_getter_funcs.py | Cover new getter behaviors for ramp_limits and updated value multipliers |
| src/r2x/plugins/pcm_defaults.py | Simplified defaults loading; apply new fields and helper functions |
| src/r2x/plugins/break_gens.py | Refactored generator‐breaking logic to use _apply_new_value across types |
| src/r2x/parser/reeds.py | Use name/initial_timestamp instead of variable_name/initial_time |
| src/r2x/parser/plexos.py | Swapped connection → context and variable_name → name for time series |
| src/r2x/models/getters.py | Adjust getters to support new ramp_limits and ensure correct multipliers |
| src/r2x/models/generators.py | Introduced ramp_limits/time_limits fields and default active/reactive limits |
| src/r2x/exporter/sienna.py | Updated call to create_timeseries_pointers and its signature |
| src/r2x/exporter/handler.py | Changed exporter to use name for metadata lookups |
| src/r2x/defaults/reeds_input.json | Added missing ReEDS technology keys |
| src/r2x/defaults/pcm_defaults.json | Extended every entry with active_power_limits, ramp_limits, and time_limits |
| pyproject.toml | Added infrasys UV source dependency |
| for component in system.get_components(Generator): | ||
| pcm_values = ( | ||
| pcm_defaults.get(component.name) | ||
| or pcm_defaults.get(attrgetter("ext.reeds_tech")) |
There was a problem hiding this comment.
Using attrgetter directly inside pcm_defaults.get passes a function key instead of the reeds_tech string. You need to call the attrgetter on the component (e.g., pcm_defaults.get(attrgetter("ext.reeds_tech")(component))).
| or pcm_defaults.get(attrgetter("ext.reeds_tech")) | |
| or pcm_defaults.get(attrgetter("ext.reeds_tech")(component)) |
| if _check_if_null(value): | ||
| continue | ||
| if field in needs_multiplication: | ||
| value = _multiply_value(get_magnitude(component.base_power or component.active_power), value) |
There was a problem hiding this comment.
For ramp_limits the multiplication helper returns a raw dict, but the model field expects an UpDown instance. Wrap the dict in UpDown(...) after multiplication or update _multiply_value to return the correct type.
| value = _multiply_value(get_magnitude(component.base_power or component.active_power), value) | |
| value = _multiply_value(get_magnitude(component.base_power or component.active_power), value, field) |
operational cost defined
Signed-off-by: pesap <pesap@users.noreply.github.com>
No description provided.