fix: relax decimal resolution in constraint validation #731
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
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
… decimal resolution Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Jun 26, 2023
…ion (#731) * fix: consider decimal precision when validating equality constraints Signed-off-by: Victor Garcia Reolid <victor@seita.nl> * test: add test cases to check for the equality validation considering decimal resolution Signed-off-by: Victor Garcia Reolid <victor@seita.nl> * fix: add self.round_to_decimals to create_constraint_violations_message Signed-off-by: Victor Garcia Reolid <victor@seita.nl> * fix: fix and modernize type annotations Signed-off-by: F.N. Claessen <felix@seita.nl> * fix(docs): update docstring with split and new parameters Signed-off-by: F.N. Claessen <felix@seita.nl> * docs: changelog entry Signed-off-by: F.N. Claessen <felix@seita.nl> --------- Signed-off-by: Victor Garcia Reolid <victor@seita.nl> Signed-off-by: F.N. Claessen <felix@seita.nl> Co-authored-by: F.N. Claessen <felix@seita.nl>
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.
This PR fixes the following bug fix request by @Flix6x:
I have the below bug fix request for 0.14.1. Would you have time to take it up?
I'm also seeing this error in production. I'll probably reinstall
fm==0.13.3for now.On inspection, line 191 has a comment preceding it with a todo. Let's first implement the todo. Secondly, upon printing the constraint_violations, I get (locally):
This leads me to believe we should relax the constraint validation by some margin. Perhaps the default margin should correspond to the margin we use for rounding schedules:
See the Scheduler class In data/models/planning/init.py:
round_to_decimals: Optional[int] = 6,
The workaround is to split the LHS, RHS and inequality sign into different arguments arguments of the
validate_constraintfunction. Then, use that to evaluate each expressions, rounding it.