Skip to content

fix: address PR #138 review comments (scenariobuilder rename + model-…#144

Merged
aoustry merged 1 commit intoclaude/update-gemspy-docs-81CmJfrom
claude/address-pr-comments-XsgHD
Apr 30, 2026
Merged

fix: address PR #138 review comments (scenariobuilder rename + model-…#144
aoustry merged 1 commit intoclaude/update-gemspy-docs-81CmJfrom
claude/address-pr-comments-XsgHD

Conversation

@aoustry
Copy link
Copy Markdown
Collaborator

@aoustry aoustry commented Apr 30, 2026

…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

…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 aoustry merged commit f714a8d into claude/update-gemspy-docs-81CmJ Apr 30, 2026
1 check passed
@aoustry aoustry deleted the claude/address-pr-comments-XsgHD branch April 30, 2026 08:27
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants