dimod icon indicating copy to clipboard operation
dimod copied to clipboard

Use a more uniform structure for cython classes

Open arcondello opened this issue 3 years ago • 1 comments

Currently we have two patterns for Cython classes and it would be nice to be more consistent

Subclass pattern

Have a Cython class as a superclass of the Python class

cdef class cyClass:
    ...

class Class(cyClass):
    ...

examples: cyVariables and Variables, cyConstrainedQuadraticModel and ConstrainedQuadraticModel

The major drawback of this pattern is it does not allow multiple data types, because Cython does not allow templated classes (nor is such a concept really meaningful).

Attribute pattern

Have the Python class hold the Cython class as an attribute

cdef class cyClass
    ...

class Class:
    def __init__(self):
        self.data = cyClass()

examples: cyBQM and BQM

This pattern gives us the ability to support multiple underlying data types, but IMO it's harder to understand and requires a lot of indirect methods like

def f(self):
    return self.data.f()

which adds clutter and maintenance and creates a performance problem (partially mititgated by forwarding_method).

arcondello avatar Nov 10 '22 19:11 arcondello

One potential alternative to the attribute pattern would be something like

import abc

import numpy as np

class BQM_float64:
    pass


class BQM_float32:
    pass


class BQM(abc.ABC):
    def __new__(cls, *args, dtype=np.float64, **kwargs):
        if dtype == np.float32:
            return BQM_float32(*args, **kwargs)
        elif dtype == np.float64:
            return BQM_float64(*args, **kwargs)
        else:
            raise ValueError

BQM.register(BQM_float64)
BQM.register(BQM_float32)

where we create a stub BQM class that just constructs one of the two other datatype classes. Which themselves can inherit from the Cython classes.

That said, this has some issues. Namely it's a bit surprising for a user, and the docs etc will break.

arcondello avatar Nov 10 '22 20:11 arcondello