Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better introspection #7

Open
hgrecco opened this issue Nov 22, 2020 · 2 comments
Open

Better introspection #7

hgrecco opened this issue Nov 22, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@hgrecco
Copy link
Owner

hgrecco commented Nov 22, 2020

Alternative 1: subclass

class Implicit:
    pass

class Explicit
    pass

class FixedStep:
    pass

class VariableStep
    pass

and then

class Solver:

    @classproperty
    def is_implicit(cls):
        return issubclass(cls, Implicit)

and the same for others.

Pros:

  • No extra definitions are needed

Cons:

  • To avoid circular imports, import within class property or uncomfortable classes organization might be needed.
  • Used need to now the classes in order to know more information (e.g. DIAGONALLY_IMPLICT)

Alternative 2: enums

Step = Enum("Step", "FIXED VARIABLE")
Implicitness = Enum("Implicitness", "EXPLICIT IMPLICIT DIAGONALLY_IMPLICT")

(names and members might differ)

In the solver, add the following attributes and classproperties

class Solver:
    IMPLICITNESS

    @classproperty
    def is_implicit(cls):
        return cls.IMPLICITNESS in (Implicitness.IMPLICIT, Implicitness.DIAGONALLY_IMPLICT)

and the same for others.

Pros:

  • Defined orthogonal to classes
  • Better discoverability

Cons:

  • Yet another hierarchy to maintainn.
@hgrecco hgrecco added the enhancement New feature or request label Nov 22, 2020
@maurosilber
Copy link
Collaborator

In the case of implicitness, it can be introspected from the coefficients, which would let us check that there are no misclassifications or typos in coefficients:

class RungeKutta:
    def is_implicit(cls):
        """The method is implicit if any coefficient of A's upper triangle is not 0."""
        return np.any(np.triu(cls.A) != 0)

class Multistep:
    def is_implicit(cls):
        """The method is implicit if Bn is not 0."""
        return cls.Bn != 0

In Multistep, IMPLICIT is implemented as a class property.

RungeKutta doesn't have that method, but each of its specializations has a class property (explicit, implicit, diagonally_implicit) which is checked by __init_subclass__.

@hgrecco
Copy link
Owner Author

hgrecco commented Nov 24, 2020

+1 on having classification done by inspecting the coefficients.
+1 on adding a is_implicit classproperty
-1 on having a explicit, implicit, diagonally_implicit, etc property as provides a non homogeneous API in different solvers. I lke the Implicitness (in certain cases set automatically at __init_class__)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants