Imath icon indicating copy to clipboard operation
Imath copied to clipboard

Add tests for baseTypeLowest/Max/Smallest/Epsilon

Open cary-ilm opened this issue 2 years ago • 3 comments

Various class templates declare methods that return lowest/max/smallest/epsilon on the template base type, e.g.

V2f::baseTypeLowest()
V2f::baseTypeMax()
V2f::baseTypeSmallest()
V2f::baseTypeEpsilon()

These methods are not covered in the test suite. Entries should be added to the python test src/python/PyImathTest/pyImathTest.in

cary-ilm avatar Sep 11 '23 23:09 cary-ilm

Hi, If I have read the codebase correctly, pyImathTest.in contains the tests specific to the python bindings of the Imath library.

And the C++ class templates methods that, return the lowest/max/smallest/epsilon values of the base type, and where these methods are binded into python, eg.

.def("baseTypeEpsilon", &Matrix44<T>::baseTypeEpsilon,"baseTypeEpsilon() epsilon value of the base type of the vector")

must be tested in python similar to:

import imath

M44dEpsilon = imath.M44d.baseTypeEpsilon() 

DBL_EPS == M44dEpsilon

Is this correct?

ghost avatar Sep 12 '23 11:09 ghost

The python-based tests have the dual advantage of validating both the binding code and the underlying Imath functionality, so the majority of the coverage of the test suite actually comes from pyImathTest.in. Your suggestion is along the lines of what I had in mind, replicated for each of the class instantiations.

I noted this issue from the SonarCloud coverage analysis report: https://sonarcloud.io/component_measures?metric=uncovered_lines&view=list&id=AcademySoftwareFoundation_Imath

A straightforward issue as a potential task for the ASWF Dev Days event.

cary-ilm avatar Sep 12 '23 16:09 cary-ilm

From what I see in the pyImathTest.in, when trying to integrate baseTypeLimits, there is a fundemental mismatch in the use of types.

We have in pyImathTest.in:

VecBaseType = {}
VecBaseType[V2s] = int
VecBaseType[V2i] = int
VecBaseType[V2f] = float
VecBaseType[V2d] = float
VecBaseType[V3s] = int
VecBaseType[V3i] = int
VecBaseType[V3f] = float
VecBaseType[V3d] = float

But, as you know, Python "float" is actually C/C++ double and we have int64_t and short in C/C++ types, which in the tests, are being treated as the intuitively closest type in Python.

However in the main codebase we have class methods similar to:

IMATH_HOSTDEVICE constexpr static T baseTypeLowest () IMATH_NOEXCEPT
    {
        return std::numeric_limits<T>::lowest ();
    }

Where T can be any one of C/C++ float, double, int, short, int64_t or even possibly unsigned char.

Without significant changes to possibly the entire pyImathTest.in and some minor additions to the imathmodule.cpp file, I don't think it is possible to correctly test the baseTypeLimits.

For instance, we need to add the below code into the imathmodule.cpp file to start:

scope().attr("SHT_MIN")      = std::numeric_limits<short>::min();
scope().attr("SHT_MAX")      = std::numeric_limits<short>::max();
scope().attr("SHT_LOWEST")   = std::numeric_limits<short>::lowest();
scope().attr("SHT_EPS")      = std::numeric_limits<short>::epsilon();

scope().attr("I64_MIN")    = std::numeric_limits<int64_t>::min();
scope().attr("I64_MAX")    = std::numeric_limits<int64_t>::max();
scope().attr("I64_LOWEST") = std::numeric_limits<int64_t>::lowest();
scope().attr("I64_EPS")    = std::numeric_limits<int64_t>::epsilon();

To give a concrete example why we would have to change the pyImathTest.in file, we have tests on various templated classes like so:

def testV2 ():

    print ("V2i")
    testV2x (V2i)
    print ("V2f")
    testV2x (V2f)
    print ("V2d")
    testV2x (V2d)

But the BaseTypes defined at te start of the file means that we will have errors in testing the limits of V2f.

ghost avatar Sep 13 '23 17:09 ghost