From 804761da884d17ec36793d55393e427337cd6a43 Mon Sep 17 00:00:00 2001 From: EvilSpirit Date: Thu, 21 Jan 2016 15:28:05 +0600 Subject: [PATCH] Distinguish overconstrained and redundantly constrained sketches. When a solver error arises after a change to the sketch, it should be easy to understand exactly why it happened. Before this change, two functionally distinct modes of failure were lumped into one: the same "redundant constraints" message was displayed when all degrees of freedom were exhausted and the had a solution, but also when it had not. To understand why this is problematic, let's examine several ways in which we can end up with linearly dependent equations in our system: 0) create a triangle, then constrain two different pairs of edges to be perpendicular 1) add two distinct distance constraints on the same segment 2) add two identical distance constraints on the same segment 3) create a triangle, then constrain edges to lengths a, b, and c so that a+b=c The case (0) is our baseline case: the constraints in it make the system unsolvable yet they do not remove more degrees of freedom than the amount we started with. So the displayed error is "unsolvable constraints". The constraints in case (1) remove one too many degrees of freedom, but otherwise are quite like the case (0): the cause of failure that is useful to the user is that the constraints are mutually incompatible. The constraints in cases (2) and (3) however are not like the others: there is a set of parameters that satisfies all of the constraints, but the constraints still remove one degree of freedom too many. It makes sense to display a different error message for cases (2) and (3) because in practice, cases like this are likely to arise from adjustment of constraint values on sketches corresponding to systems that have a small amount of degenerate solutions, and this is very different from systems arising in cases like (0) where no adjustment of constraint values will ever result in a successful solution. So the error message displayed is "redundant constraints". At last, this commit makes cases (0) and (1) display a message with only a minor difference in wording. This is deliberate. The reason is that the facts "the system is unsolvable" and "the system is unsolvable and also has linearly dependent equations" present no meaningful, actionable difference to the user, and placing emphasis on it would only cause confusion. However, they are still distinguished, because in case (0) we list all relevant constraints (and thus we say they are "mutually incompatible") but in case (1) we only list the ones that constrain the sketch further than some valid solution (and we say they are "unsatisfied"). --- src/lib.cpp | 3 ++- src/solvespace.h | 10 ++++++---- src/system.cpp | 29 +++++++++++++++++------------ src/textscreens.cpp | 7 ++++++- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/lib.cpp b/src/lib.cpp index ac179d8..cb9fca2 100644 --- a/src/lib.cpp +++ b/src/lib.cpp @@ -220,7 +220,8 @@ default: dbp("bad constraint type %d", sc->type); return; ssys->result = SLVS_RESULT_DIDNT_CONVERGE; break; - case System::REDUNDANT: + case System::REDUNDANT_DIDNT_CONVERGE: + case System::REDUNDANT_OKAY: ssys->result = SLVS_RESULT_INCONSISTENT; break; diff --git a/src/solvespace.h b/src/solvespace.h index 38c671e..9f94a7b 100644 --- a/src/solvespace.h +++ b/src/solvespace.h @@ -417,6 +417,7 @@ public: static const double RANK_MAG_TOLERANCE, CONVERGE_TOLERANCE; int CalculateRank(void); + bool TestRank(void); static bool SolveLinearSystem(double X[], double A[][MAX_UNKNOWNS], double B[], int N); bool SolveLeastSquares(void); @@ -433,10 +434,11 @@ public: bool NewtonSolve(int tag); enum { - SOLVED_OKAY = 0, - DIDNT_CONVERGE = 10, - REDUNDANT = 11, - TOO_MANY_UNKNOWNS = 20 + SOLVED_OKAY = 0, + DIDNT_CONVERGE = 10, + REDUNDANT_OKAY = 11, + REDUNDANT_DIDNT_CONVERGE = 12, + TOO_MANY_UNKNOWNS = 20 }; int Solve(Group *g, int *dof, List *bad, bool andFindBad, bool andFindFree); diff --git a/src/system.cpp b/src/system.cpp index 154d269..2773303 100644 --- a/src/system.cpp +++ b/src/system.cpp @@ -172,6 +172,11 @@ int System::CalculateRank(void) { return rank; } +bool System::TestRank(void) { + EvalJacobian(); + return CalculateRank() == mat.m; +} + bool System::SolveLinearSystem(double X[], double A[][MAX_UNKNOWNS], double B[], int n) { @@ -194,7 +199,7 @@ bool System::SolveLinearSystem(double X[], double A[][MAX_UNKNOWNS], // Don't give up on a singular matrix unless it's really bad; the // assumption code is responsible for identifying that condition, // so we're not responsible for reporting that error. - if(ffabs(max) < 1e-20) return false; + if(ffabs(max) < 1e-20) continue; // Swap row imax with row i for(jp = 0; jp < n; jp++) { @@ -216,7 +221,7 @@ bool System::SolveLinearSystem(double X[], double A[][MAX_UNKNOWNS], // We've put the matrix in upper triangular form, so at this point we // can solve by back-substitution. for(i = n - 1; i >= 0; i--) { - if(ffabs(A[i][i]) < 1e-20) return false; + if(ffabs(A[i][i]) < 1e-20) continue; temp = B[i]; for(j = n - 1; j > i; j--) { @@ -398,6 +403,8 @@ int System::Solve(Group *g, int *dof, List *bad, WriteEquationsExceptFor(Constraint::NO_CONSTRAINT, g); int i, j = 0; + bool rankOk; + /* dbp("%d equations", eq.n); for(i = 0; i < eq.n; i++) { @@ -446,20 +453,18 @@ int System::Solve(Group *g, int *dof, List *bad, return System::TOO_MANY_UNKNOWNS; } + rankOk = TestRank(); + // And do the leftovers as one big system if(!NewtonSolve(0)) { goto didnt_converge; } - EvalJacobian(); - - int rank; rank = CalculateRank(); - if(rank != mat.m) { - if(andFindBad) { - FindWhichToRemoveToFixJacobian(g, bad); - } - return System::REDUNDANT; + if(!TestRank()) { + if(andFindBad) FindWhichToRemoveToFixJacobian(g, bad); + return System::REDUNDANT_OKAY; } + // This is not the full Jacobian, but any substitutions or single-eq // solves removed one equation and one unknown, therefore no effect // on the number of DOF. @@ -477,7 +482,7 @@ int System::Solve(Group *g, int *dof, List *bad, p->tag = VAR_DOF_TEST; WriteJacobian(0); EvalJacobian(); - rank = CalculateRank(); + int rank = CalculateRank(); if(rank == mat.m) { p->free = true; } @@ -522,7 +527,7 @@ didnt_converge: } } - return System::DIDNT_CONVERGE; + return rankOk ? System::DIDNT_CONVERGE : System::REDUNDANT_DIDNT_CONVERGE; } void System::Clear(void) { diff --git a/src/textscreens.cpp b/src/textscreens.cpp index 9e811f5..807113e 100644 --- a/src/textscreens.cpp +++ b/src/textscreens.cpp @@ -471,11 +471,16 @@ void TextWindow::ShowGroupSolveInfo(void) { Printf(true, "%FtGROUP %E%s", g->DescriptionString().c_str()); switch(g->solved.how) { case System::DIDNT_CONVERGE: + Printf(true, "%FxSOLVE FAILED!%Fd unsolvable constraints"); + Printf(true, "the following constraints are incompatible"); + break; + + case System::REDUNDANT_DIDNT_CONVERGE: Printf(true, "%FxSOLVE FAILED!%Fd unsolvable constraints"); Printf(true, "the following constraints are unsatisfied"); break; - case System::REDUNDANT: + case System::REDUNDANT_OKAY: Printf(true, "%FxSOLVE FAILED!%Fd redundant constraints"); Printf(true, "remove any one of these to fix it"); break;