fix: address PR #138 review comments (scenariobuilder rename + model-…#144
Merged
aoustry merged 1 commit intoclaude/update-gemspy-docs-81CmJfrom Apr 30, 2026
Merged
Conversation
…decomposition docs) - Rename scenariobuilder.dat to modeler-scenariobuilder.dat and move from input/ to input/data-series/ for consistency with Antares study format - Add explicit study directory tree to inputs.md - Document model-decomposition key in optim-config.md with location values table and YAML example - Update all test fixtures, test references, docstrings, and docs accordingly https://claude.ai/code/session_01CdMYwXn5ECs9Hcuq9uvBYw
aoustry
added a commit
that referenced
this pull request
Apr 30, 2026
* Implement scenario builder: vectorized MC scenario → column dispatch (issue #101) - ScenarioBuilder.load() parses scenariobuilder.dat (group, mc_scenario = time_serie_number format; 1-based columns stored as 0-based internally) into per-group numpy arrays for loop-free resolution via resolve_vectorized(). - DataBase.get_values() resolves MC scenarios → col_idx at use time (vectorized, no Python loop over S) and dispatches to the underlying data object in one call. - get_value() on all data classes vectorized: scenario arg is now np.ndarray of col_idx; ScenarioSeriesData stores scenario_series as np.ndarray for O(1) indexing. - Scenarization removed from data objects (resolution now lives in DataBase). - build_data_base() accepts optional ScenarioBuilder and records scenario_group per parameter; build_scenarized_data_base() and _resolve_scenarization() deleted. - load_study() passes ScenarioBuilder to build_data_base() so the full study path is wired. - optimization.py is now unaware of ScenarioBuilder/scenario_group: the isinstance + for-s_pos loops are replaced by a single database.get_values() call per component. - Tests: existing unit/e2e tests updated to new API; new dispatch beacon test added. https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Fix mypy, black and isort issues - folder.py: load ScenarioBuilder before passing it to build_data_base() (fixes used-before-definition mypy error) - parsing.py, optimization.py, test_scenario_builder_dispatch.py: apply black 23.7 and isort formatting https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Fix test_data_consistency: update ScenarioSeriesData construction to np.ndarray ScenarioSeriesData.scenario_series changed from Mapping[ScenarioIndex, float] to np.ndarray in the scenario builder refactor. Update the two direct instantiations in test_data_consistency.py to pass np.array([100, 50]) and remove the now-unused ScenarioIndex import. https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Fix shape broadcasting when data type doesn't cover all requested dimensions TimeSeriesData returns (T,) but optimization.py may request (T, S) when a time-only parameter is used in a time+scenario problem. Likewise ScenarioSeriesData returns (S,) but may be assigned into a (T, S) slot. Broadcast to the correct shape so optimization.py can unconditionally write data[i, :, :] = v. https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Return broadcast view instead of copy to avoid materialising (T,S) array The broadcast view is read-only but optimization.py only reads from it (data[i,:,:] = v), so no copy is needed. Memory cost drops from T*S elements to T or S elements respectively. https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Use np.asarray and direct .values[] access to avoid unnecessary copies - TimeSeriesData: replace iloc[array].to_numpy() with .values[asarray()]; the iloc path allocated an intermediate pandas Series before copying to numpy; direct .values[] indexing skips that object entirely - TimeScenarioSeriesData / DataBase.get_values(): np.array() forces a copy even when the input is already an ndarray; np.asarray() avoids that The np.ix_ fancy-index copy (TimeScenarioSeriesData) and the scenario_series fancy-index copy (ScenarioSeriesData) are unavoidable for arbitrary index selection and are left as-is. https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Apply black formatting to data.py https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Raise ValueError for unknown scenario group instead of silently falling back resolve_vectorized() previously returned the identity mapping when a scenario_group was named but not present in the builder, masking misconfiguration (typo in system.yml, missing entry in scenariobuilder.dat). Now: None → identity (parameter has no group, correct); non-None but missing → ValueError with the group name and list of known groups. https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * Fix test_optim_modes CI failure: remove spurious scenario-group declarations (#135) The dsr_3_blocks study declared scenario-group: sg on all 7 components but had no scenariobuilder.dat file to define that group. This was silently ignored by the old identity-fallback behaviour, but now raises a ValueError after 70c526e enforced strict validation. Since scenario mapping is irrelevant to these optimisation-mode tests (single scenario, single-column data series), simply drop the declarations. https://claude.ai/code/session_01EDyJsnw5zAQGkWAVAruCjc Co-authored-by: Claude <noreply@anthropic.com> * scenario_builder: raise ValueError on incomplete MC scenario mapping (#136) Validates at load time that every integer in 0..max_mc is explicitly present in the scenariobuilder.dat for each group. Previously a missing entry would silently fall through to the identity default (col_idx == mc_scenario), hiding misconfigured files. https://claude.ai/code/session_016CVWfhLdoDHaiMbTM45iwv Co-authored-by: Claude <noreply@anthropic.com> * Guard resolve_vectorized against out-of-bounds MC scenario indices; add tests A numpy IndexError on out-of-bounds access gave no context about which group or which indices were problematic. Add an explicit check that raises ValueError naming the offending indices and the defined range. Tests added: - playlist (subset) resolves correctly - out-of-bounds index raises ValueError - unknown group raises ValueError https://claude.ai/code/session_018xbwEtc3QCZ7jGVBoi8fMe * docs: adapt documentation to post-Feb-2026 API changes - Fix swapped library/system file variables in getting-started.md example - Rename all Input* schema classes to *Schema throughout (ComponentSchema, ComponentParameterSchema, PortConnectionsSchema, SystemSchema) - Replace removed build_network() call; resolve_system() now returns System directly - Update build_problem() signature: scenarios: int → scenario_ids: List[int] - Replace OR-Tools solver API (problem.solver.Solve() / Objective().Value()) with linopy API (problem.solve() / problem.objective_value) - Add load_study() and run_study() as recommended directory-based entry points - Add SimulationSession as mid-level API in optimisation.md - Document SimulationTable fluent accessor API and export methods in outputs.md - Add new page: user-guide/optim-config.md covering OptimConfig YAML structure, all four resolution modes (frontal, sequential, parallel, benders), and out-of-bounds constraint processing - Add new page: user-guide/scenario-builder.md covering scenariobuilder.dat format, scenario_group on components, and ScenarioBuilder Python API - Update mkdocs.yml navigation to include both new pages https://claude.ai/code/session_01PLuVqJXEtvXyBSSvM2vaZP * Remove usage notes for optimization configurations Removed usage guidelines for optimization modes. * Update cyclic mode description in optim-config.md Clarify the description of the 'cyclic' mode behavior in the optim-config documentation. * Clarify building systems with GemsPy API * Rename parameter 'spread_cost' to 'spillage_cost' * fix: address PR #138 review comments (scenariobuilder rename + model-decomposition docs) (#144) - Rename scenariobuilder.dat to modeler-scenariobuilder.dat and move from input/ to input/data-series/ for consistency with Antares study format - Add explicit study directory tree to inputs.md - Document model-decomposition key in optim-config.md with location values table and YAML example - Update all test fixtures, test references, docstrings, and docs accordingly https://claude.ai/code/session_01CdMYwXn5ECs9Hcuq9uvBYw Co-authored-by: Claude <noreply@anthropic.com> * Claude/review documentation lr4 vt (#146) * fix: correct major documentation errors found in review - Fix broken `from gems.study import load_study` in four doc files; load_study lives in gems.study.folder and was never re-exported - Export TimeScopeConfig, ScenarioScopeConfig, SolverOptionsConfig from gems.optim_config so the documented import path works without error - Fix scenario-builder.md: mc_scenario indices are 0-based in the .dat file, not 1-based as previously stated; update example and note - Fix AGENTS.md: install command (no requirements files, use uv sync), run_study location (gems.study.runner not folder), run_study signature (no scenarios/time_block params), ResolutionMode default (FRONTAL) - Fix developer-guide.md: stale pip/requirements-dev.txt install step https://claude.ai/code/session_014djdbYUFdAVMmjHWt7nAdR * docs: update GEMS acronym expansion in AGENTS.md https://claude.ai/code/session_014djdbYUFdAVMmjHWt7nAdR * docs: expose GEMS acronym expansion on the introductive page https://claude.ai/code/session_014djdbYUFdAVMmjHWt7nAdR --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <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.
…decomposition docs)
https://claude.ai/code/session_01CdMYwXn5ECs9Hcuq9uvBYw