Reduce asset_is_realized big-M safety margin to 1.0#503
Open
jarsarasty wants to merge 1 commit into
Open
Conversation
Since big_m in financial mixin already uses actual bounds, extra margin was unnecessary
jarsarasty
requested review from
FJanssen-TNO,
KobusVanRooyen and
jimrojerTNO
as code owners
July 9, 2026 15:49
FJanssen-TNO
left a comment
Collaborator
There was a problem hiding this comment.
@jarsarasty Review is ready, just a small comment.
| logger = logging.getLogger("mesido") | ||
|
|
||
| # No safety factor is needed for big_m, since actual bounds are used. | ||
| ASSET_IS_REALIZED_BIG_M_MARGIN = 1.0 |
Collaborator
There was a problem hiding this comment.
In a perfect world indeed no safety factor is needed for big_m. However this big_m margin is also used for a heatflow that is divided by the aggregationcount, here previously some rounding errors have caused issues once in a while (not always easy to reproduce due to different solvers or even order of rows in the matrices). A safety factor of 1.5 is indeed overkill and might give other (scaling) issues. However I would prefer a small safety margin, for instance changing this ASSET_IS_REALIZED_BIG_MARGIN to 1.01. This should prevent round errors to select the wrong integer, while not messing up the scaling.
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.
The
asset_is_realizedbig-M constraints infinancial_mixin.py(__cumulative_investments_made_in_eur_path_constraintsand__cumulative_capex_made_in_eur_constraints) computedbig_mfrom the asset's own registered cost/Heat_flow bounds, then applied an extra1.5xsafety margin on top. Since the underlying bound is already exact, this margin was unnecessary.Numeric literals for safety margin replaced with a named constant.