Skip to content

Commit 18fbd99

Browse files
Isaac Garzonruevs
authored andcommitted
solver: remove the rank parameter from Solve() and fix SolveRank()
It wasn't set inside `System::Solve()`, and it's not useful anyway when rank testing is fully or partially suppressed. OTOH, in `System::SolveRank()` it is useful, and although it wasn't set, it was actually checked in `Constraint::TryConstrain()` (through the call to `SolveSpaceUI::TestRankForGroup()`), so fix it by letting `System::TestRank()` set it if provided.
1 parent 27d1d11 commit 18fbd99

5 files changed

Lines changed: 12 additions & 12 deletions

File tree

js/slvs.d.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ export interface Constraint {
2727
export interface SolveResult {
2828
result: number;
2929
dof: number;
30-
rank: number;
3130
bad: number;
3231
}
3332

src/generate.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,8 +536,7 @@ void SolveSpaceUI::SolveGroup(hGroup hg, bool andFindFree) {
536536
Group *g = SK.GetGroup(hg);
537537
g->solved.remove.Clear();
538538
g->solved.findToFixTimeout = SS.timeoutRedundantConstr;
539-
SolveResult how = sys.Solve(g, NULL,
540-
&(g->solved.dof),
539+
SolveResult how = sys.Solve(g, &(g->solved.dof),
541540
&(g->solved.remove),
542541
/*andFindBad=*/!g->allowRedundant,
543542
/*andFindFree=*/andFindFree,

src/slvs/lib.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -854,8 +854,8 @@ Slvs_SolveResult Slvs_SolveSketch(uint32_t shg, int calculateFaileds = 0)
854854
List<hConstraint> badList;
855855
bool andFindBad = calculateFaileds ? true : false;
856856

857-
int rank = 0, dof = 0;
858-
SolveResult status = SYS.Solve(&g, &rank, &dof, &badList, andFindBad, false, false);
857+
int dof = 0;
858+
SolveResult status = SYS.Solve(&g, &dof, &badList, andFindBad, false, false);
859859
Slvs_SolveResult sr = {};
860860
sr.dof = dof;
861861
sr.bad = badList.n;
@@ -982,7 +982,7 @@ void Slvs_Solve(Slvs_System *ssys, uint32_t shg)
982982

983983
// Now we're finally ready to solve!
984984
bool andFindBad = ssys->calculateFaileds ? true : false;
985-
SolveResult how = SYS.Solve(&g, NULL, &(ssys->dof), &bad, andFindBad, /*andFindFree=*/false);
985+
SolveResult how = SYS.Solve(&g, &(ssys->dof), &bad, andFindBad, /*andFindFree=*/false);
986986

987987
switch(how) {
988988
case SolveResult::OKAY:

src/solvespace.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ class System {
268268

269269
static const double CONVERGE_TOLERANCE;
270270
int CalculateRank();
271-
bool TestRank(int *dof = NULL);
271+
bool TestRank(int *dof = NULL, int *rank = NULL);
272272
static bool SolveLinearSystem(const Eigen::SparseMatrix<double> &A,
273273
const Eigen::VectorXd &B, Eigen::VectorXd *X);
274274
bool SolveLeastSquares();
@@ -287,8 +287,7 @@ class System {
287287

288288
void MarkParamsFree(bool findFree);
289289

290-
SolveResult Solve(Group *g, int *rank = NULL, int *dof = NULL,
291-
List<hConstraint> *bad = NULL,
290+
SolveResult Solve(Group *g, int *dof = NULL, List<hConstraint> *bad = NULL,
292291
bool andFindBad = false, bool andFindFree = false,
293292
bool forceDofCheck = false);
294293

src/system.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,15 @@ int System::CalculateRank() {
229229
return result;
230230
}
231231

232-
bool System::TestRank(int *dof) {
232+
bool System::TestRank(int *dof, int *rank) {
233233
EvalJacobian();
234234
int jacobianRank = CalculateRank();
235235
// We are calculating dof based on real rank, not mat.m.
236236
// Using this approach we can calculate real dof even when redundant is allowed.
237237
if(dof != NULL) *dof = mat.n - jacobianRank;
238+
if(rank) {
239+
*rank = jacobianRank;
240+
}
238241
return jacobianRank == mat.m;
239242
}
240243

@@ -414,7 +417,7 @@ void System::FindWhichToRemoveToFixJacobian(Group *g, List<hConstraint> *bad, bo
414417
}
415418
}
416419

417-
SolveResult System::Solve(Group *g, int *rank, int *dof, List<hConstraint> *bad,
420+
SolveResult System::Solve(Group *g, int *dof, List<hConstraint> *bad,
418421
bool andFindBad, bool andFindFree, bool forceDofCheck)
419422
{
420423
WriteEquationsExceptFor(Constraint::NO_CONSTRAINT, g);
@@ -546,7 +549,7 @@ SolveResult System::SolveRank(Group *g, int *rank, int *dof, List<hConstraint> *
546549
return SolveResult::TOO_MANY_UNKNOWNS;
547550
}
548551

549-
bool rankOk = TestRank(dof);
552+
bool rankOk = TestRank(dof, rank);
550553
if(!rankOk) {
551554
// When we are testing with redundant allowed, we don't want to have additional info
552555
// about redundants since this test is working only for single redundant constraint

0 commit comments

Comments
 (0)