Skip to content

Fix two bugs in ApprPolynomial4::Evaluate#117

Open
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/appr-polynomial4-evaluate
Open

Fix two bugs in ApprPolynomial4::Evaluate#117
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/appr-polynomial4-evaluate

Conversation

@CodeReclaimers
Copy link
Contributor

Two bugs in the Horner evaluation of ApprPolynomial4::Evaluate():

  1. Line 249: writes to mYZCoefficient with stride mYDegree instead of mYDegreeP1, causing z-slice overlap.
  2. Line 259: reads from mParameters instead of mYZCoefficient in the y-Horner loop, discarding the x-evaluation from stage 1.

Both bugs are masked when xDegree == 0. For xDegree >= 1 and yDegree >= 1, evaluation results are wrong.

Test (CMakeLists.txt)
cmake_minimum_required(VERSION 3.14)
project(test_issue_2_2_2_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_2_2_2_3 test_issue_2_2_2_3.cpp)
target_include_directories(test_issue_2_2_2_3 PRIVATE "${GTE_ROOT}")
Test (test_issue_2_2_2_3.cpp)
#include <cmath>
#include <cstdlib>
#include <iostream>
#include <array>
#include <vector>
#include <Mathematics/ApprPolynomial4.h>

static bool test_trilinear_evaluation()
{
    using Real = double;
    auto poly = [](Real x, Real y, Real z) -> Real {
        return 1.0 + 2.0*x + 3.0*y + 4.0*z
             + 5.0*x*y + 6.0*x*z + 7.0*y*z + 8.0*x*y*z;
    };

    gte::ApprPolynomial4<Real> fitter(1, 1, 1);
    std::vector<std::array<Real, 4>> obs;
    for (Real x = 0.0; x <= 1.0; x += 0.25)
        for (Real y = 0.0; y <= 1.0; y += 0.25)
            for (Real z = 0.0; z <= 1.0; z += 0.25)
                obs.push_back({ x, y, z, poly(x, y, z) });

    fitter.Fit(obs.size(), obs.data());
    Real x = 0.37, y = 0.61, z = 0.83;
    return std::fabs(fitter.Evaluate(x, y, z) - poly(x, y, z)) < 1e-10;
}

static bool test_quadratic_xy_linear_z()
{
    using Real = double;
    auto poly = [](Real x, Real y, Real z) -> Real {
        return 1.0 + 2.0*x + 3.0*y + 4.0*z + 5.0*x*x + 6.0*y*y
             + 7.0*x*y + 8.0*x*z + 9.0*y*z + 10.0*x*x*y + 11.0*x*y*y
             + 12.0*x*x*z + 13.0*y*y*z + 14.0*x*y*z + 15.0*x*x*y*z
             + 16.0*x*y*y*z + 17.0*x*x*y*y + 18.0*x*x*y*y*z;
    };

    gte::ApprPolynomial4<Real> fitter(2, 2, 1);
    std::vector<std::array<Real, 4>> obs;
    for (Real x = 0.0; x <= 1.0; x += 0.1)
        for (Real y = 0.0; y <= 1.0; y += 0.1)
            for (Real z = 0.0; z <= 1.0; z += 0.1)
                obs.push_back({ x, y, z, poly(x, y, z) });

    fitter.Fit(obs.size(), obs.data());
    Real x = 0.42, y = 0.73, z = 0.55;
    return std::fabs(fitter.Evaluate(x, y, z) - poly(x, y, z)) < 1e-8;
}

int main()
{
    int bugs = (!test_trilinear_evaluation() ? 1 : 0)
             + (!test_quadratic_xy_linear_z() ? 1 : 0);
    return bugs > 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}

Line 249: wrong stride (mYDegree instead of mYDegreeP1) when writing
to mYZCoefficient, causing z-slice overlap.

Line 259: reads from mParameters instead of mYZCoefficient in the
y-Horner loop, discarding the x-evaluation from stage 1.

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