Feature/element data classes#602
Draft
FBumann wants to merge 395 commits into
Draft
Conversation
Summary A) Fixed statistics accessor variable categories - Root cause: get_variables_by_category() was returning batched variable names (e.g., flow|rate) instead of unrolled per-element names (e.g., Boiler(Q_th)|flow_rate) - Fix: Modified get_variables_by_category() in flow_system.py to always expand batched variables to unrolled element names - Additional fix: For FLOW_SIZE category, now only returns flows with InvestParameters (not fixed-size flows that have NaN values) B) Removed EffectCollection.submodel pattern - Removed the dead submodel: EffectCollectionModel | None attribute declaration from EffectCollection class - EffectCollectionModel itself is kept since it's actively used as a coordination layer for effects modeling (wraps EffectsModel, handles objective function, manages cross-effect shares) Files Modified - flixopt/flow_system.py - Fixed get_variables_by_category() logic - flixopt/effects.py - Removed dead submodel attribute Test Results - All 91 clustering tests pass - All 13 statistics tests pass - All 194 storage/component/flow/effect tests pass - All 30 integration/functional tests pass
1. Coordinate Building Helper (_build_coords)
- Enhanced TypeModel._build_coords() to accept optional element_ids and extra_timestep parameters
- Simplified coordinate building in:
- FlowsModel._add_subset_variables() (elements.py)
- BusesModel._add_subset_variables() (elements.py)
- StoragesModel.create_variables() (components.py)
- InterclusterStoragesModel - added the method and simplified create_variables()
2. Investment Effects Mixin (previously completed)
- InvestmentEffectsMixin consolidates 5 shared cached properties used by FlowsModel and StoragesModel
3. Concat Utility (concat_with_coords)
- Created concat_with_coords() helper in features.py
- Replaces repeated xr.concat(...).assign_coords(...) pattern
- Used in 8 locations across:
- components.py (5 usages)
- features.py (1 usage)
- elements.py (1 usage)
4. StoragesModel Inheritance
- Updated StoragesModel to inherit from both InvestmentEffectsMixin and TypeModel
- Removed duplicate dim_name property (inherited from TypeModel)
- Simplified initialization using super().__init__()
Code Reduction
- ~50 lines removed across coordinate building patterns
- Consistent patterns across all type-level models
- Better code reuse through mixins and utility functions
1. Categorizations as cached properties with with_* naming:
- with_status → list[str] of flow IDs with status parameters
- with_investment → list[str] of flow IDs with investment
- with_optional_investment → list[str] of flow IDs with optional investment
- with_mandatory_investment → list[str] of flow IDs with mandatory investment
- with_flow_hours_over_periods → list[str] of flow IDs with that constraint
2. Lookup helper:
- flow(label: str) -> Flow - get Flow object by ID
3. Dicts as cached properties:
- _flows_by_id → cached dict for fast lookup
- _invest_params → cached dict of investment parameters
- _status_params → cached dict of status parameters
- _previous_status → cached dict of previous status arrays
4. Lean __init__:
- Only calls super().__init__() and sets flow references
- All categorization and dict building is lazy via cached properties
5. Updated constraint methods:
- _create_status_bounds(), _create_investment_bounds(), _create_status_investment_bounds() now accept list[str] (flow IDs) instead of list[Flow]
FlowsData (batched.py):
1. Added categorizations: with_flow_hours, with_load_factor
2. Renamed: size_minimum → effective_size_lower, size_maximum → effective_size_upper
3. Properties now only include relevant flows (no NaN padding):
- flow_hours_minimum/maximum → only with_flow_hours
- flow_hours_minimum/maximum_over_periods → only with_flow_hours_over_periods
- load_factor_minimum/maximum → only with_load_factor
4. Added absolute_lower_bounds, absolute_upper_bounds for all flows
5. Added _stack_values_for_subset() helper
FlowsModel (elements.py):
1. Removed hours and hours_over_periods variables - not needed
2. Simplified constraints to compute inline:
- constraint_flow_hours() - directly constrains sum_temporal(rate)
- constraint_flow_hours_over_periods() - directly constrains weighted sum
- constraint_load_factor_min/max() - compute hours inline
3. rate variable uses self.data.absolute_lower_bounds/upper_bounds directly
4. Removed obsolete bound collection methods
Benefits:
- Cleaner separation: data in FlowsData, constraints in FlowsModel
- No NaN handling needed - properties only include relevant flows
- Fewer variables in the model
- More explicit about which flows have which constraints
…_to_model_coords = fit_to_model_coords up to right after self.clusters = clusters, before any _fit_data() calls. Removed the duplicate assignment that was at line 86.
…ontained ASCII diagrams or numbered step-lists (lines 9, 445, 459, 524, 542). This satisfies markdownlint MD040 which requires all fenced blocks to have a language tag. 1. Fenced block language tag — Changed the opening fence around the variable count comparison (line 19) from ``` to ```text. 2. Incorrect variable name — Replaced all occurrences of storage|charge_state with storage|charge (lines 28, 81, 121, 135). The actual variable defined in structure.py:264 is CHARGE = 'storage|charge' — charge_state is only used for intercluster storages
…e) if value else np.nan — when value=0, this returned NaN instead of 0.0. Changed to return float(value) since None and array cases are already handled above.
… called np.isnan() directly, which raises TypeError for integer or object arrays. Added a try/except fallback to pd.isnull() for those dtypes. Also added import pandas as pd.
…ame matches "Bus1" inside "Bus10". Changed to element_id in con_name.split('|') for delimiter-aware exact matching.
2. Lines 487-493 — Boolean mask becomes float. mask.reindex() with NaN fill turns booleans to float. Added fill_value=False to the reindex call and mask = mask.astype(bool) after expand_dims to keep the dtype boolean.
…red element names but didn't reset the model's _is_built flag, so get_status() would still report MODEL_BUILT. Added fs.model._is_built = False when fs.model is not None.
…taArray() (a scalar with no dims) when empty, breaking downstream .dims checks. Changed to xr.DataArray(dims=['case'], coords={'case': []}) so the 'case' dimension is always present.
- PiecewiseBuilder.create_piecewise_constraints — removed the zero_point parameter entirely. It now always creates sum(inside_piece) <= 1. - Callers (FlowsModel and StoragesModel) — add the tighter <= invested constraint separately, only for optional IDs that exist in invested_var. No coord mismatch possible. - ConvertersModel — was already passing None, just cleaned up the dead code.
…erseded/math/ directory. Here's a summary of the changes made:
Summary of Updated Tests
test_flow.py (88 tests)
- Updated variable names: flow|rate, flow|size, flow|invested, flow|status, flow|active_hours
- Updated constraint names: share|temporal(costs), share|periodic(costs) instead of 'ComponentName->effect(temporal)'
- Updated uptime/downtime constraints: flow|uptime|forward, flow|uptime|backward, flow|uptime|min instead of flow|uptime|fwd/bwd/lb
- Updated switch constraints: flow|switch_transition instead of flow|switch
- Removed non-existent flow|fixed constraint check (fixed profile uses flow|invest_lb/ub)
test_storage.py (48 tests)
- Updated variable names: storage|charge, storage|netto, storage|size, storage|invested
- Updated constraint names: storage|balance, storage|netto_eq, storage|initial_equals_final
- Updated status variable from status|status to flow|status
- Updated prevent simultaneous constraint: storage|prevent_simultaneous
- Fixed effects_of_investment syntax to use dict: {'costs': 100}
test_component.py (40 tests)
- Updated status variables: component|status, flow|status instead of status|status
- Updated active_hours variables: component|active_hours
- Updated uptime variables: component|uptime
- Updated constraints: component|status|lb/ub/eq, component|uptime|initial
- Removed non-existent flow|total_flow_hours checks
test_linear_converter.py (36 tests)
- Updated constraint names: converter|conversion (no index suffix)
- Updated status variables: component|status, component|active_hours
- Updated share constraints: share|temporal(costs)
- Made piecewise tests more flexible with pattern matching
test_effect.py (26 tests)
- No changes needed - tests already working
1. Storage charge state scalar bounds (batched.py): Added .astype(float) after expand_dims().copy() to prevent silent int→float truncation when assigning final charge state overrides (0.5 was being truncated to 0 on an int64 array). 2. SourceAndSink deserialization (components.py): Convert inputs/outputs from dict to list before + concatenation in __init__, fixing TypeError: unsupported operand type(s) for +: 'dict' and 'dict' during NetCDF save/reload. 3. Legacy config leaking between test modules (test_math/conftest.py, superseded/math/conftest.py, test_legacy_solution_access.py): Converted module-level fx.CONFIG.Legacy.solution_access = True to autouse fixtures that restore the original value after each test, preventing the plotting isinstance(solution, xr.Dataset) test from failing.
Member
Author
Test Comparison:
|
| main | feature | |
|---|---|---|
| Passed | 1638 | 1383 |
| Skipped | 3 | 99 |
| Deselected | 6 | 0 |
| Failed | 0 | 0 |
| Warnings | 216 | 698 |
| Total collected | 1647 | 1482 |
| Time | 19m 08s | 5m 05s |
Test Count Difference (-165 total)
The difference is entirely due to test file restructuring — no coverage was lost:
- 22 files removed from
tests/deprecated/(596 tests on main) - 11 files added as
tests/superseded/math/+ new test files (431 tests) - Net: -165 tests from reorganization
Skipped (3 → 99)
The +96 skipped tests are the tsam-dependent superseded math tests (importorskip('tsam')).
Warnings (216 → 698)
- ResourceWarning: 39 → 7 (fewer because deprecated
test_results_plots.pywith plotly socket warnings was removed) - DeprecationWarning: increased because
test_math/tests use theoptimizefixture parametrized 3 ways, tripling legacy solution access warnings. All expected.
Performance
Feature branch is ~4x faster (5m vs 19m) — the removed deprecated tests were slow functional tests.
CI Failures (not related to this branch's changes)
- Tests (3.11/3.12/3.13):
test_full_scenario_optimizationfails with gurobi license error ("Model too large for size-limited license") — infrastructure issue, not a code bug - Docs build:
tsam==3.0.0was yanked upstream; dependency constrainttsam>=3.0.0,<3.1.0needs updating to include3.1.0
* Here's a summary of everything that was done: Summary Phase 1: TransmissionsData - Added flow_ids parameter to TransmissionsData.__init__ - Moved 12 cached properties from TransmissionsModel to TransmissionsData: bidirectional_ids, balanced_ids, _build_flow_mask(), in1_mask, out1_mask, in2_mask, out2_mask, relative_losses, absolute_losses, has_absolute_losses_mask, transmissions_with_abs_losses - Updated TransmissionsModel.create_constraints() to use self.data.* - Updated BatchedAccessor.transmissions to pass flow_ids Phase 2: BusesData - Added balance_coefficients cached property to BusesData - Updated BusesModel.create_constraints() to use self.data.balance_coefficients Phase 3: ConvertersData - Added flow_ids and timesteps parameters to ConvertersData.__init__ - Moved 13 cached properties from ConvertersModel to ConvertersData: factor_element_ids, max_equations, equation_mask, signed_coefficients, n_equations_per_converter, piecewise_element_ids, piecewise_segment_counts_dict, piecewise_max_segments, piecewise_segment_mask, piecewise_flow_breakpoints, piecewise_segment_counts_array, piecewise_breakpoints - Updated ConvertersModel methods to use self.data.* - Removed unused defaultdict and stack_along_dim imports from elements.py Phase 4: ComponentsData - Added flows_data, effect_ids, timestep_duration parameters to ComponentsData.__init__ - Moved 6 cached properties from ComponentsModel to ComponentsData: with_prevent_simultaneous, status_params, previous_status_dict, status_data, flow_mask, flow_count - Moved _get_previous_status_for_component() helper to ComponentsData - Updated ComponentsModel to use self.data.* throughout Bug Fix - Discovered a stale cache issue: FlowsData.previous_states could be cached before all previous_flow_rate values were set (e.g., in from_old_results). Fixed by having ComponentsData._get_previous_status_for_component() compute previous status directly from flow attributes instead of going through the potentially-stale FlowsData.previous_states cache. * 1. _build_flow_mask exposure: Added balanced_in1_mask and balanced_in2_mask cached properties to TransmissionsData. TransmissionsModel now uses these instead of calling the private d._build_flow_mask(). 2. EffectsModel/elements: Confirmed not a bug — EffectsModel does not inherit from TypeModel and never accesses .elements on its data, so EffectsData not having elements is fine. 3. FlowsData.dim_name: Changed from @cached_property to @Property to match all other Data classes.
…classes # Conflicts: # CHANGELOG.md # tests/deprecated/test_config.py
- StatusData.with_effects_per_active_hour and StatusData.with_effects_per_startup — categorization lists that were never accessed. The actual effect values (effects_per_active_hour, effects_per_startup) are applied correctly via a different path. Removed passthrough properties (components.py — StoragesModel) 9 properties eliminated, replaced with direct self.data.X at all call sites: - with_investment, with_optional_investment, with_mandatory_investment - storages_with_investment, storages_with_optional_investment - optional_investment_ids, mandatory_investment_ids - invest_params, _investment_data Kept investment_ids — it has 4 external callers in optimization.py. Removed passthrough property (elements.py — FlowsModel) - _previous_status → replaced 3 call sites with self.data.previous_states
# Conflicts: # CHANGELOG.md # flixopt/io.py # flixopt/statistics_accessor.py # flixopt/transform_accessor.py # tests/deprecated/conftest.py # tests/deprecated/examples/03_Optimization_modes/example_optimization_modes.py # tests/test_clustering/test_cluster_reduce_expand.py # tests/test_math/test_clustering.py
- stats.flow_sizes/storage_sizes return empty arrays when the solution has no size variables instead of raising KeyError - transform.fix_sizes() accepts the element-dim DataArray that batched stats.sizes returns by splitting it into per-element variables - cyclic clustered-storage SOC assertions made degeneracy-robust (both the level and the charge timing are solver-dependent, see #733) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tests - from_dataset: connect the network before restoring the solution — restoring first sets status SOLVED, which made connect_and_transform() skip and left bus wiring empty, crashing any rebuild of a reloaded system - solution expansion tolerates FlowSystems without a solution - main-era clustering tests adapted to batched solution variable names - manual SOC decay comparison gets an atol for near-zero values Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Description
Major refactoring of the model building pipeline to use batched/vectorized operations instead of per-element loops. This brings significant performance improvements, especially for large models.
Key Changes
Batched Type-Level Models: New
FlowsModel,StoragesModel,BusesModelclasses that handle ALL elements of a type in single batched operations instead of individualFlowModel,StorageModelinstances.FlowsData/StoragesData Classes: Pre-compute and cache element data as xarray DataArrays with element dimensions, enabling vectorized constraint creation.
Mask-based Variable Creation: Variables use linopy's
mask=parameter to handle heterogeneous elements (e.g., only some flows have status variables) while keeping consistent coordinates.Fast NumPy Helpers: Replace slow xarray methods with numpy equivalents:
fast_notnull()/fast_isnull()- ~55x faster than xarray's.notnull()/.isnull()Unified Coordinate Handling: All variables use consistent coordinate order via
.reindex()to prevent alignment errors.Performance Results
Scaling Summary
The batched approach provides 7-32x build speedup depending on model size, with the benefit growing as models get larger.
Scaling by Number of Converters
Base config: 720 timesteps, 1 period, 2 effects, 5 storages
Main scales O(n) with converters (168→1,688 vars), while the feature branch stays constant at 15 vars. Build time on main grows ~11x for 20x more converters; the feature branch grows only ~1.7x.
Scaling by Number of Effects
Base config: 720 timesteps, 1 period, 50 converters (102 flows), each flow contributes to ALL effects
The batched approach handles effect share constraints in O(1) instead of O(n_effects × n_flows). Main grows 7.5x for 20x effects; the feature branch grows only 1.7x.
Scaling by Number of Storages
Base config: 720 timesteps, 1 period, 2 effects, 50 converters
Same pattern: main scales O(n) with storages while the feature branch stays constant.
Scaling by Timesteps and Periods
Speedup remains consistent (~8-12x) regardless of time horizon or period count.
XL System End-to-End (2000h, 300 converters, 50 storages)
Model Size Reduction
The batched approach creates fewer, larger variables instead of many small ones:
Why This Matters
The old approach creates one linopy Variable per flow/storage element. Each creation has ~1ms overhead, so 200 converters × 2 flows = 400 variables = 400ms just for variable creation. Constraints are created per-element in loops.
The new approach creates one batched Variable with an element dimension. A single
flow|ratevariable contains ALL flows in one DataArray, and constraints use vectorized xarray operations with masks. Variable count stays constant regardless of model size.Type of Change
Testing
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Breaking Changes