The default lp-degree of MultiIndexSet is not transparently set
Consider the default constructor for a MultiIndexSet
instance:
def __init__(self, exponents: ARRAY, lp_degree=None, poly_deg_dtype=None)
and the class method constructor from_degree
:
def from_degree(cls, spatial_dimension: int, poly_degree: int, lp_degree=None):
In both cases, lp_degree
is set by default to None
.
But then inside the body of these methods, the utility function verify_lp_deg
is called.
What this function does is actually set the default None
to the "global" default lp-degree of 2.0 (DEFAULT_LP_DEG
):
def verify_lp_deg(lp_degree):
if lp_degree is None:
lp_degree = DEFAULT_LP_DEG
return lp_degree
I guess, this function actually does more than verifying (checking types, bounds, etc. which it doesn't actually do) and it is not transparent in setting the default lp-degree.
This is potentially problematic; There may be cases in which the lp-degree may be mistakenly not specified but then stealthily set to 2.0 as this value does not appear in the constructors signature.
I think lp-degree should not have a default at all and has to be explicitly specified (that would save us from potential problems) when creating multi-index set,
but I understand that 2.0 may be the most optimal choice of lp-degree in many interpolation tasks so it should be used as the default.
What can be improved, in my opinion, is to be more transparent and straightforward when setting this default; we shouldn't say None
as the default lp-degree when creating multi-index but then is "verified" to be 2.0 if it is None
.
Note that verify_lp_deg
is only used by the MultiIndexSet
constructors.
Perhaps there is a particular reason of setting the default this way?