Conversation
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 93 |
| Duplication | 1 |
🟢 Coverage 97.41% diff coverage
Metric Results Coverage variation Report missing for 98870801 Diff coverage ✅ 97.41% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (9887080) Report Missing Report Missing Report Missing Head commit (809336f) 26055 21349 81.94% Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#757) 695 677 97.41% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
|
BDonnot
commented
Sep 23, 2026
BDonnot
left a comment
BDonnot
left a comment
Collaborator
Author
There was a problem hiding this comment.
some changes to be made
Comment on lines
+171
to
+227
| except_ = None | ||
| cls = type(self.env) | ||
| this_dt_float = float | ||
| new_p = constraints.new_p | ||
| if self.env.nb_time_step == 0: | ||
| state.gen_activeprod_t_redisp[:] = new_p | ||
|
|
||
| gen_participating = constraints.gen_participating.copy() | ||
| incr_in_chronics = new_p - (state.gen_activeprod_t_redisp - state.actual_dispatch) | ||
|
|
||
| p_min_down = cls.gen_pmin[gen_participating] - state.gen_activeprod_t_redisp[gen_participating] | ||
| avail_down = np.maximum(p_min_down, -cls.gen_max_ramp_down[gen_participating]) | ||
| p_max_up = cls.gen_pmax[gen_participating] - state.gen_activeprod_t_redisp[gen_participating] | ||
| avail_up = np.minimum(p_max_up, cls.gen_max_ramp_up[gen_participating]) | ||
| except_ = self._detect_infeasible_dispatch( | ||
| constraints, | ||
| incr_in_chronics[gen_participating], | ||
| avail_down, | ||
| avail_up, | ||
| state, | ||
| ) | ||
| if except_ is not None: | ||
| if ( | ||
| self.env._parameters.IGNORE_MIN_UP_DOWN_TIME | ||
| and self.env._parameters.ALLOW_DISPATCH_GEN_SWITCH_OFF | ||
| ): | ||
| gen_participating_tmp = self.env.gen_redispatchable.copy() | ||
| if cls.detachment_is_allowed: | ||
| gen_participating_tmp[constraints.gen_detached] = False | ||
| p_min_down_tmp = ( | ||
| cls.gen_pmin[gen_participating_tmp] | ||
| - state.gen_activeprod_t_redisp[gen_participating_tmp] | ||
| ) | ||
| avail_down_tmp = np.maximum( | ||
| p_min_down_tmp, -cls.gen_max_ramp_down[gen_participating_tmp] | ||
| ) | ||
| p_max_up_tmp = ( | ||
| cls.gen_pmax[gen_participating_tmp] | ||
| - state.gen_activeprod_t_redisp[gen_participating_tmp] | ||
| ) | ||
| avail_up_tmp = np.minimum( | ||
| p_max_up_tmp, cls.gen_max_ramp_up[gen_participating_tmp] | ||
| ) | ||
| except_tmp = self._detect_infeasible_dispatch( | ||
| constraints, | ||
| incr_in_chronics[gen_participating_tmp], | ||
| avail_down_tmp, | ||
| avail_up_tmp, | ||
| state, | ||
| ) | ||
| if except_tmp is None: | ||
| gen_participating = gen_participating_tmp | ||
| except_ = None | ||
| else: | ||
| return except_tmp | ||
| else: | ||
| return except_ |
Collaborator
Author
There was a problem hiding this comment.
All this part deserves its function, even in baseResdispatchSolver call something like _prepare_solver_inputs or something
Comment on lines
+229
to
+243
| target_vals = state.target_dispatch[gen_participating] - state.actual_dispatch[gen_participating] | ||
| already_modified_gen_me = state.already_modified_gen[gen_participating] | ||
| target_vals_me = target_vals[already_modified_gen_me] | ||
| nb_dispatchable = gen_participating.sum() | ||
| tmp_zeros = np.zeros((1, nb_dispatchable), dtype=this_dt_float) | ||
| coeffs = 1.0 / (self.env.gen_max_ramp_up + self.env.gen_max_ramp_down + self.env._epsilon_poly) | ||
| weights = np.ones(nb_dispatchable) * coeffs[gen_participating] | ||
| weights /= weights.sum() | ||
|
|
||
| if target_vals_me.shape[0] == 0: | ||
| already_modified_gen_me[:] = True | ||
| target_vals_me = target_vals[already_modified_gen_me] | ||
|
|
||
| scale_x = max(np.max(np.abs(state.actual_dispatch)), 1.0) | ||
| scale_x = this_dt_float(scale_x) |
Collaborator
Author
There was a problem hiding this comment.
This should also leave in its own function, for this class only though, called "scale_solver_input" or something
Comment on lines
107
to
110
| - [IMPROVED] handling of redispatching as a separate module now | ||
| (grid2op/Environment/dispatch) | ||
| - [IMPROVED] remove the use of "assert" block in the main codebase | ||
|
|
Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
The environment owns the redispatching rules again: tracking the target dispatch, validating redispatching actions and the min up / down times of the generators are back in BaseEnv. A solver only has to implement solve() (reset() is optional), so a subclass of BaseRedispatchSolver that implements the abstract API no longer crashes on env.reset(). - RedispatchConstraints.from_state carries everything the solver needs (limits, tolerances, whether all generators may be used), so the default solver no longer reads the environment. It is built only when a dispatch has to be computed. - review: the participating generators / feasibility check live in BaseRedispatchSolver._prepare_solver_inputs, the scaling in DefaultRedispatchSolver._scale_solver_input. - redispatch_solver is accepted by grid2op.make (and config.py) and is propagated to obs.simulate, the forecast env, env.copy(), the Runner, MultiMix, MaskedEnvironment and TimedOutEnvironment. Each env works on its own copy of the solver. - the fallback that uses every redispatchable generator copied gen_redispatchable instead of modifying the class attribute in place. - remove the BaseEnv wrappers that no longer had any caller. - tests: test_redispatch_solver.py (custom solver smoke test and a regression test for gen_redispatchable, which fails on dev_1.12.6). - docs: document the redispatch solver in docs/user/environment.rst. Assisted-by: Claude Code Claude-Session: https://claude.ai/code/session_01SjJybpBWCD2gf6AJ1AFKfD Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
They were under 1.12.5, which is already released (review comment). Also drop the "remove assert" line (already listed in 1.12.5) and add the entries for the gen_redispatchable fix and the new redispatch_solver argument. Assisted-by: Claude Code Claude-Session: https://claude.ai/code/session_01SjJybpBWCD2gf6AJ1AFKfD Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
…ctions without redispatch - simulate: _reset_to_orig_state restored _detached_elements_mw_prev from the "_detached_elements_mw" key. The environment used by simulate also never knew the power of the detached loads (its injections are in _backend_action_set, not _env_modification). The two errors cancelled on steady steps, but simulate on the observation right after a detachment predicted a dispatch that no longer compensated the detached load. - LIMIT_INFEASIBLE_CURTAILMENT_STORAGE_ACTION: the feasibility guard now uses the same generators as the solver (detached ones excluded) and counts the detached power, so a storage action that is only infeasible together with a detachment is limited instead of causing a game over. What the guard takes back is capped at what storage and curtailment contributed (they are cancelled, never reversed), a full cancellation now updates the storage power, and the state of charge uses the right efficiency in that case. - the injections of an action were overridden by the time series on grids without redispatching data and without storage units (_aux_handle_act_inj was only called in _aux_apply_redisp). Regression tests in test_redispatch_solver.py fail before these changes. Assisted-by: Claude Code Claude-Session: https://claude.ai/code/session_01SjJybpBWCD2gf6AJ1AFKfD Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
The forecast env performs an internal step when it is created. Its backend is a copy of the one of the observation (so detached elements are already detached) but the state of the environment (dispatch, generator setpoints, detached power...) was only restored from the observation in `reset`, not before this first step. With a detached element the dispatch then saw the whole production as a variation from 0 MW and raised ImpossibleRedispatching. Restore the state of the observation before this first step too, as reset does. Assisted-by: Claude Code Claude-Session: https://claude.ai/code/session_01SjJybpBWCD2gf6AJ1AFKfD Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
Solver contract changes, done before the API is released: - RedispatchConstraints carries a single power_to_compensate_mw (positive: the generators must produce more) instead of the storage, curtailment and detachment amounts, each in its own sign convention. The split is kept, in that same convention, in `contributions`, for information. Both come from dispatch_contributions(), the only place a new source (load shedding, deferred loads...) has to be declared. - solve() returns a RedispatchResult (success, the new actual_dispatch, the exception, and unserved_mw: the power the generators cannot compensate) and no longer modifies the state: the environment applies the dispatch. A solver returning anything else raises an EnvError. The default solver computes the same dispatch (the power to compensate is summed in the same order and precision as the former equality constraint). Assisted-by: Claude Code Claude-Session: https://claude.ai/code/session_01SjJybpBWCD2gf6AJ1AFKfD Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
This branch has not been deployed
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.
No description provided.