Skip to content

Fix GMatrix::operator!= returning false for different dimensions#115

Open
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/gmatrix-operator-ne-different-dimensions
Open

Fix GMatrix::operator!= returning false for different dimensions#115
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/gmatrix-operator-ne-different-dimensions

Conversation

@CodeReclaimers
Copy link
Contributor

Problem

GMatrix::operator!= (line 217) requires dimensions to match AND elements to differ. When dimensions differ, it returns false — meaning a 2x2 and 3x3 matrix are considered "equal" by !=.

Fix

 inline bool operator!=(GMatrix const& mat) const
 {
-    return mNumRows == mat.mNumRows && mNumCols == mat.mNumCols
-        && mElements != mat.mElements;
+    return !operator==(mat);
 }

Test program

Before fix:

Test 1 (different dims):    BUG DETECTED
Test 4 (consistency):       BUG DETECTED

After fix:

Test 1 (different dims):    passed
Test 2 (diff values):       passed
Test 3 (identical):         passed
Test 4 (consistency):       passed
CMakeLists.txt
cmake_minimum_required(VERSION 3.14)
project(test_issue_1_3 LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

set(GTE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../GTE")

add_executable(test_issue_1_3 test_issue_1_3.cpp)
target_include_directories(test_issue_1_3 PRIVATE "${GTE_ROOT}")
test_issue_1_3.cpp
// Test for Issue 1.3: GMatrix.h operator!= returns false for different dimensions
//
// Bug: operator!= requires dimensions to match AND elements to differ.
// When dimensions don't match it returns false, meaning a 2x2 matrix
// is considered "equal" to a 3x3 matrix by !=.
//
// File: GTE/Mathematics/GMatrix.h, lines 217-221
//   return mNumRows == mat.mNumRows && mNumCols == mat.mNumCols
//       && mElements != mat.mElements;
//
// Expected fix: return !operator==(mat);

#include <cstdlib>
#include <iostream>

#include <Mathematics/GMatrix.h>

static bool test_different_dimensions()
{
    std::cout << "Test 1: operator!= with different dimensions\n";

    gte::GMatrix<double> a(2, 2);
    gte::GMatrix<double> b(3, 3);
    a.MakeZero();
    b.MakeZero();

    // These are clearly different matrices. operator!= should return true.
    bool ne_result = (a != b);
    bool eq_result = (a == b);

    std::cout << "  2x2 matrix vs 3x3 matrix:\n";
    std::cout << "    operator== returns " << (eq_result ? "true" : "false") << "\n";
    std::cout << "    operator!= returns " << (ne_result ? "true" : "false") << "\n";

    if (!ne_result)
    {
        std::cout << "  FAIL: operator!= says a 2x2 and 3x3 matrix are equal.\n";
        return false;
    }
    std::cout << "  PASS: operator!= correctly returns true.\n";
    return true;
}

static bool test_same_dimensions_different_values()
{
    std::cout << "\nTest 2: operator!= with same dimensions, different values\n";

    gte::GMatrix<double> a(2, 2);
    gte::GMatrix<double> b(2, 2);
    a.MakeZero();
    b.MakeZero();
    b(0, 0) = 1.0;

    bool ne_result = (a != b);
    if (!ne_result)
    {
        std::cout << "  FAIL: operator!= says different matrices are equal.\n";
        return false;
    }
    std::cout << "  PASS: operator!= correctly returns true.\n";
    return true;
}

static bool test_same_dimensions_same_values()
{
    std::cout << "\nTest 3: operator!= with identical matrices (baseline)\n";

    gte::GMatrix<double> a(2, 3);
    gte::GMatrix<double> b(2, 3);
    a.MakeZero();
    b.MakeZero();
    a(0, 1) = 5.0;
    b(0, 1) = 5.0;

    bool ne_result = (a != b);
    if (ne_result)
    {
        std::cout << "  FAIL: operator!= says identical matrices are different.\n";
        return false;
    }
    std::cout << "  PASS: operator!= correctly returns false.\n";
    return true;
}

static bool test_consistency()
{
    std::cout << "\nTest 4: operator!= is always !operator==\n";

    struct TestCase { int r1, c1, r2, c2; };
    TestCase cases[] = {
        {2, 2, 3, 3},  // different dimensions
        {2, 3, 2, 4},  // same rows, different cols
        {2, 2, 2, 2},  // same dimensions
    };

    bool all_ok = true;
    for (auto const& tc : cases)
    {
        gte::GMatrix<double> a(tc.r1, tc.c1);
        gte::GMatrix<double> b(tc.r2, tc.c2);
        a.MakeZero();
        b.MakeZero();

        bool eq = (a == b);
        bool ne = (a != b);

        if (ne != !eq)
        {
            all_ok = false;
            std::cout << "  FAIL: (" << tc.r1 << "x" << tc.c1 << ") vs ("
                      << tc.r2 << "x" << tc.c2 << "): "
                      << "== returns " << eq << ", != returns " << ne << "\n";
        }
    }

    if (all_ok)
        std::cout << "  PASS: operator!= is always !operator== for all cases.\n";
    return all_ok;
}

int main()
{
    std::cout << "=== Issue 1.3: GMatrix operator!= wrong for different dimensions ===\n\n";

    bool ok1 = test_different_dimensions();
    bool ok2 = test_same_dimensions_different_values();
    bool ok3 = test_same_dimensions_same_values();
    bool ok4 = test_consistency();

    std::cout << "\n=== Summary ===\n";
    std::cout << "Test 1 (different dims):    " << (ok1 ? "passed" : "BUG DETECTED") << "\n";
    std::cout << "Test 2 (diff values):       " << (ok2 ? "passed" : "BUG DETECTED") << "\n";
    std::cout << "Test 3 (identical):         " << (ok3 ? "passed" : "BUG DETECTED") << "\n";
    std::cout << "Test 4 (consistency):       " << (ok4 ? "passed" : "BUG DETECTED") << "\n";

    int bugs = (!ok1?1:0) + (!ok2?1:0) + (!ok3?1:0) + (!ok4?1:0);
    std::cout << "\n" << bugs << " bug(s) detected out of 4 tests.\n";
    return bugs > 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}

🤖 Generated with Claude Code

operator!= required dimensions to match AND elements to differ, so
two matrices of different sizes were considered "equal." Fix by
defining it as !operator==.

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