Unfeasible problem analyzer : add a new check #1689

guilpier-code · 2023-10-06T15:08:45Z

The current PR to fix issue #1682

what's done here :

Make a refactoring before welcoming the change (add a new analysis)
- Remove exceptions catching a problem in the analyzer itself. Removing theses exceptions simplifies the code.
- Simplify analyzer's interface to simplify the client calls.
- split analyzer into : an algorithm that manages a collection of analyses and the slack variables analysis. Note that split and separation makes code more testable (in isolation).
- Currently, when slack variables analysis cannot detect the unfeasibiliy's reason, it prints an error message. This can be misleading for users because it's not exactly an error but an inability of one analysis to catch the unfeasibility's reason. There may be another analysis that can catch it. So these messages can be printed as warnings or notice.
- Separate the analysis manager and every analysis in different sources files
- add some unit tests : test each class in isolation (units) and add one or two integration tests to check everything work together
- what else ?
add the new check on consistency of each (original) variable bounds

…vion

…n() function for analysis

…nto a shared pointer This change should be useful in the devs to come, when passing the problem to new analysis classes

…tion

…eport by adding a sort method

…rt class

…ve slack vars analysis to its own class

…m abstract analysis class

…ounds analysis

…implementing the run() method

…this title is then printed to logs

…implementing the printReport() method

…de for further unit tests

…e files

…undsConsistency in separate source files

… and any particular analysis as dependency of analyzer

…pointer

sylvlecl

a few more design issue + unit tests would be nice :)

src/solver/infeasible-problem-analysis/report.h

src/solver/optimisation/opt_appel_solveur_lineaire.cpp

sylvlecl · 2023-10-20T12:50:10Z

src/solver/infeasible-problem-analysis/unfeasibility-analysis.h

+
+namespace Antares::Optimization
+{
+class UnfeasibilityAnalysis


We could add some documentation comments on the few classes we added, to clarify their role

src/solver/optimisation/opt_appel_solveur_lineaire.cpp

sylvlecl · 2023-10-20T12:57:04Z

src/solver/infeasible-problem-analysis/variables-bounds-consistency.h

+    double upBound;
+};
+
+class VariablesBoundsConsistency : public UnfeasibilityAnalysis


Can we add unit tests to this module, like a few tests for each component, the 2 analysis and the analyzer?

Yes we can. It should be the subject of a further issue / PR.

Will be done in #1716

src/solver/infeasible-problem-analysis/unfeasible-pb-analyzer.h

src/solver/infeasible-problem-analysis/constraint-slack-analysis.h

src/solver/infeasible-problem-analysis/variables-bounds-consistency.cpp

…run methods

…run methods (next part)

src/solver/infeasible-problem-analysis/constraint-slack-analysis.h

a-zakir · 2023-10-24T13:00:03Z

Not that i'm against it but the new file's name are not consistent with other lib name convention (we generally don't have hyphens)

src/solver/infeasible-problem-analysis/unfeasible-pb-analyzer.cpp

…feasible package

…ared ptr with unique ptr

…copy into a const &

…tions that need it

Signed-off-by: Sylvain Leclerc <[email protected]>

sonarqubecloud · 2023-11-03T12:49:51Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
7 Code Smells

46.6% Coverage
0.0% Duplication

guilpier-code added 4 commits October 6, 2023 15:02

New log msg when solver not found in or-tools

b885cea

New log msg when solver not found in or-tools : oops, correct an obli…

f9e1645

…vion

Add a check to unfeasible analyzer : just a renaming

093477b

Add a check to unfeasible analyzer : remove exceptions + add a new ru…

44f0698

…n() function for analysis

guilpier-code added feature code quality labels Oct 6, 2023

guilpier-code added this to the v8.7.1 milestone Oct 6, 2023

guilpier-code requested review from sylvlecl, flomnes, a-zakir and payetvin October 6, 2023 15:08

guilpier-code self-assigned this Oct 6, 2023

pull-request-size bot added the size/L label Oct 6, 2023

flomnes added the enhancement New feature or request label Oct 6, 2023

a-zakir approved these changes Oct 6, 2023

View reviewed changes

guilpier-code added 6 commits October 9, 2023 11:02

Add a check to unfeasible analyzer : renaming slack variables

4d1d1e8

Add a check to unfeasible analyzer : merely change a unique pointer i…

4b59b50

…nto a shared pointer This change should be useful in the devs to come, when passing the problem to new analysis classes

Add a check to unfeasible analyzer : add some analysis classes defini…

f76e266

…tion

Add a check to unfeasible analyzer : clarify class InfeasibleProblemR…

4a54f05

…eport by adding a sort method

Add a check to unfeasible analyzer : clarify class InfeasibleProblemR…

83cc237

…eport by adding a sort method

Add a check to unfeasible analyzer : adding a using namespace in repo…

6ad43bd

…rt class

flomnes modified the milestones: v8.7.1, v8.7.2 Oct 9, 2023

guilpier-code added 7 commits October 10, 2023 09:34

Add a check to unfeasible analyzer : simplify analyzer interface + mo…

e0675e2

…ve slack vars analysis to its own class

Add a check to unfeasible analyzer : remove a useless data member fro…

2e1982e

…m abstract analysis class

Add a check to unfeasible analyzer : adding an incomplete variables b…

ddee96f

…ounds analysis

Add a check to unfeasible analyzer : class VariablesBoundsAnalysis - …

73994aa

…implementing the run() method

Add a check to unfeasible analyzer : add a title for every analysis, …

22dcde8

…this title is then printed to logs

Add a check to unfeasible analyzer : class VariablesBoundsAnalysis - …

3659da0

…implementing the printReport() method

[skip ci] Add a check to unfeasible analyzer : add comments in the co…

465173f

…de for further unit tests

guilpier-code added 7 commits October 18, 2023 17:50

Add a check to unfeasible analyzer : renaming file and classes

273a6c6

Add a check to unfeasible analyzer : renaming classes and source files

4b455fc

Add a check to unfeasible analyzer : renaming class and related sourc…

2691f18

…e files

Add a check to unfeasible analyzer : correct compilation and CI errors

7dc22fb

Add a check to unfeasible analyzer : mainly extract class VariablesBo…

4bffb5f

…undsConsistency in separate source files

Add a check to unfeasible analyzer : removing PROBLEME_SIMPLEXE_NOMME…

4564bc0

… and any particular analysis as dependency of analyzer

Add a check to unfeasible analyzer : turning a shared_ptr into a raw …

7ee755a

…pointer

guilpier-code requested review from sylvlecl, a-zakir and flomnes October 20, 2023 12:15

sylvlecl requested changes Oct 20, 2023

View reviewed changes

guilpier-code added 2 commits October 23, 2023 14:59

Add a check to unfeasible analyzer : make problem an argument of the …

7a1d0d5

…run methods

Add a check to unfeasible analyzer : make problem an argument of the …

0499708

…run methods (next part)

a-zakir reviewed Oct 24, 2023

View reviewed changes

src/solver/infeasible-problem-analysis/constraint-slack-analysis.h Outdated Show resolved Hide resolved

src/solver/infeasible-problem-analysis/constraint-slack-analysis.h Outdated Show resolved Hide resolved

a-zakir reviewed Oct 24, 2023

View reviewed changes

src/solver/infeasible-problem-analysis/unfeasible-pb-analyzer.cpp Outdated Show resolved Hide resolved

a-zakir reviewed Oct 24, 2023

View reviewed changes

src/solver/infeasible-problem-analysis/unfeasible-pb-analyzer.cpp Outdated Show resolved Hide resolved

guilpier-code added 4 commits October 24, 2023 16:52

Add a check to unfeasible analyzer : add a factory function to the un…

1439a4d

…feasible package

Add a check to unfeasible analyzer : in the analysis list, replace sh…

40cba9a

…ared ptr with unique ptr

Add a check to unfeasible analyzer : class VariableBounds > change a …

12c425c

…copy into a const &

Add a check to unfeasible analyzer : adding const identifiers to func…

7590b48

…tions that need it

sylvlecl mentioned this pull request Oct 25, 2023

Add check in unfeasible analyzer: unit tests #1716

Merged

guilpier-code requested review from a-zakir and sylvlecl October 25, 2023 08:10

a-zakir approved these changes Oct 25, 2023

View reviewed changes

Unit tests for unfeasible problem analyzer (#1716)

a9c4ad7

Signed-off-by: Sylvain Leclerc <[email protected]>

sylvlecl approved these changes Nov 3, 2023

View reviewed changes

sylvlecl merged commit 2242747 into develop Nov 3, 2023
7 checks passed

sylvlecl deleted the feature/add_check_in_unfeasible_analyzer branch November 3, 2023 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unfeasible problem analyzer : add a new check #1689

Unfeasible problem analyzer : add a new check #1689

guilpier-code commented Oct 6, 2023 •

edited by sylvlecl

Loading

sylvlecl left a comment

sylvlecl Oct 20, 2023

sylvlecl Oct 20, 2023

guilpier-code Oct 23, 2023

sylvlecl Oct 25, 2023

a-zakir commented Oct 24, 2023

sonarqubecloud bot commented Nov 3, 2023

Unfeasible problem analyzer : add a new check #1689

Unfeasible problem analyzer : add a new check #1689

Conversation

guilpier-code commented Oct 6, 2023 • edited by sylvlecl Loading

sylvlecl left a comment

Choose a reason for hiding this comment

sylvlecl Oct 20, 2023

Choose a reason for hiding this comment

sylvlecl Oct 20, 2023

Choose a reason for hiding this comment

guilpier-code Oct 23, 2023

Choose a reason for hiding this comment

sylvlecl Oct 25, 2023

Choose a reason for hiding this comment

a-zakir commented Oct 24, 2023

sonarqubecloud bot commented Nov 3, 2023

guilpier-code commented Oct 6, 2023 •

edited by sylvlecl

Loading