core: ignore impossible allowance constraints #7520
Merged
Conversation
This file contains 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
eckter
force-pushed
the
ech/ignore-allowance-errors
branch
from
a900f6c to
f6aa1d6
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #7520 +/- ##
=========================================
Coverage 29.36% 29.37%
- Complexity 2012 2014 +2
=========================================
Files 1200 1200
Lines 147377 147400 +23
Branches 2889 2891 +2
=========================================
+ Hits 43283 43297 +14
- Misses 102393 102401 +8
- Partials 1701 1702 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Erashin
approved these changes
May 27, 2024
eckter
force-pushed
the
ech/ignore-allowance-errors
branch
from
f6aa1d6 to
62b8665
Compare
eckter
force-pushed
the
ech/ignore-allowance-errors
branch
2 times, most recently
from
4a1412e to
b441f8a
Compare
In this case the user wants a result that's realistic and as close to the input times as possible, rather than an error. This is in line with how we ignore impossible scheduled points. We would ideally raise a warning here, but that's not supported yet.
eckter
force-pushed
the
ech/ignore-allowance-errors
branch
from
b441f8a to
416d8ef
Compare
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.
In this case the user wants a result that's realistic and as close to the input times as possible, rather than an error. This is in line with how we ignore impossible scheduled points. It should fix a significant amount of regression in the basic import (caused by a bug in the previous version that would result in too little ranges).
We would ideally raise a warning here, but that's not supported yet (see #6925).
The implementation isn't super clean, but this code is meant to be replaced soon-ish by the simulation v3. This behavior is in line with what's been designed for the v3 (warnings instead of errors).
It triggers #5778 in a regression test (which was a known bug for a long time and is unrelated). #7529 should be merged first for the CI to pass.