Skip to content

Fix assertion bugs in LDLTDecomposition.h#138

Open
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/ldlt-assertion-checks
Open

Fix assertion bugs in LDLTDecomposition.h#138
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/ldlt-assertion-checks

Conversation

@CodeReclaimers
Copy link
Contributor

Summary

  • Solve(): D.GetNumCols() was missing == N comparison, always passing for any nonzero column count
  • Block LDLT Convert(): block matrix size validated against NumBlocks instead of BlockSize

Test plan

Test passes wrong-sized inputs and verifies the assertions fire. Both tests FAIL before the fix and PASS after.

Test code (click to expand)
#include <Mathematics/LDLTDecomposition.h>
#include <cstdlib>
#include <iostream>
#include <stdexcept>

using namespace gte;

bool test_issue_7_2()
{
    LDLTDecomposition<double> decomp(3);

    GMatrix<double> A(3, 3);
    A.MakeZero();
    A(0, 0) = 4.0;
    A(1, 1) = 5.0;
    A(2, 2) = 6.0;

    GMatrix<double> L, D;
    decomp.Factor(A, L, D);

    // D with MORE columns than expected (5 instead of 3).
    // Using more columns so D(r,r) for r=0..2 are valid accesses,
    // isolating the assertion bug from any later out-of-bounds access.
    GMatrix<double> D_wrong(3, 5);
    D_wrong.MakeZero();
    D_wrong(0, 0) = 4.0;
    D_wrong(1, 1) = 5.0;
    D_wrong(2, 2) = 6.0;

    GVector<double> B(3);
    B[0] = 1.0; B[1] = 2.0; B[2] = 3.0;
    GVector<double> X;

    // Before fix: D.GetNumCols() evaluates as bool(5) = true, passes.
    // After fix:  D.GetNumCols() == N evaluates as (5 == 3) = false, throws.
    try
    {
        decomp.Solve(L, D_wrong, B, X);
        std::cerr << "FAIL 7.2: accepted D with wrong column count" << std::endl;
        return false;
    }
    catch (std::runtime_error&)
    {
        std::cout << "PASS 7.2" << std::endl;
        return true;
    }
}

bool test_issue_7_3()
{
    // BlockSize=2, NumBlocks=3 (they differ).
    BlockLDLTDecomposition<double> decomp(2, 3);

    // Blocks of WRONG size: 3x3 (== NumBlocks) instead of 2x2 (== BlockSize).
    BlockLDLTDecomposition<double>::BlockMatrix wrongBlocks(9);
    for (auto& block : wrongBlocks)
    {
        block.SetSize(3, 3);
        block.MakeZero();
    }

    GMatrix<double> M;

    // Before fix: validates rows == NumBlocks (3 == 3), passes.
    // After fix:  validates rows == BlockSize (3 == 2), throws.
    try
    {
        decomp.Convert(wrongBlocks, M);
        std::cerr << "FAIL 7.3: accepted blocks with wrong dimensions" << std::endl;
        return false;
    }
    catch (std::runtime_error&)
    {
        std::cout << "PASS 7.3" << std::endl;
        return true;
    }
}

int main()
{
    bool pass = true;
    pass = test_issue_7_2() && pass;
    pass = test_issue_7_3() && pass;
    return pass ? EXIT_SUCCESS : EXIT_FAILURE;
}

🤖 Generated with Claude Code

- Solve(): D.GetNumCols() was missing the `== N` comparison, evaluating
  as boolean (true for any nonzero column count) instead of validating
  the expected dimension.
- Block LDLT: validated block matrices against NumBlocks instead of
  BlockSize, accepting incorrectly-sized blocks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments