Naming conventions

[tlunet]

I noticed that pySDC has its own naming style for classes and modules, that is lower case for classes and CamelCase for modules, even if some classes use CamelCase (for instance, exceptions) and some module use lower case (for instance, helpers).

It is not really following the python standard (see https://peps.python.org/pep-0008/#prescriptive-naming-conventions), and I wonder if this is a deliberate choice that should be followed or not (since I already used the Python standards for the Collocation, NodesGenerator and LagrangeApproximation classes, but still kept the pySDC module naming ...).

Should we try to choose one given convention and stick on it ?

PS : as you can imagine, I would strongly suggest using Python standards 😅

PPS : I also acknowledge that I don't always follow Python standards, since I'm very found of the mixedCase for naming variables and functions. But I will comply to what the community decides

[pancetta]

Absolutely no deliberate choice, just poor me being unaware of these things back in the days. I'd rather prefer to follow standards whenever possible and I would have done so, if I only new (before it was "too late").

[pancetta]

Btw. the same applies to type checking. If I had to write pySDC now again, I'd do type checking e.g. with mypy right from the start.

[tlunet]

Ok, then I'll start writing a first draft of a CONTRIBUTING.md (if there is not already one ?) and add a first list of naming convention to it.

About the type checking, I'm a bit mixed on that. Yes Python has this feature now, but I thing it brings more constrains to the language, which is not really the spirit of it. For instance, if you expect an int (to get a size, or else), then other equivalent types will be discriminated. For instance :

a = np.array([1,2,3])  # => dtype=np.int64
type(a[0]) is int  # => False
isinstance(a[0], int)  # => False

I would more advocate the approach that uses the parameters until something break, and raise the corresponding error when its happen, for instance

class Problem():
    def __init__(self, nDOF):
        try:
            self.xValues = range(nDOF)
        except Exception:
            raise ValueError(f'wrong format for nDOF ({nDOF})')

Or the more flexible approach : the user is free to do his own mistakes 😅

[pancetta]

So, this sounds like classical duck typing, right? Ask for forgiveness instead of permission. I'm in principle ok with that, but keep in mind that certain parameters may not "break" something, but just deliver weird results. Think: provide a list instead of a tuple by coincidence, then pySDC might run MLSDC instead of SDC without you realizing it (not a good example, I know). That is something we really need to prevent, by all means.

[tlunet]

I see, passing lists in parameters is the pySDC way to instantiate multiple level by the controller (just remembering now 😅). That is nice indeed, but currently the constructor of some problems may bring some confusion. For instance, if we use the advectionNd (cf #244), we can instantiate the controller using the following problem_params dictionaries :

problem_params['nvars'] = [16, 16, 16]  # => MLSDC with the same space resolution on each level
problem_params['nvars'] = (16, 16, 16)  # => SDC for a 3D problem

I do feel weird about this, not sure what would be best (keeping this or trying to find an other way ... 😕)

[tlunet]

PS : actually, one idea to explicitly ask for multiple levels would be to use a dedicated class inherited from list (or tuple if we want to freeze it), that is passed as controller argument to be more explicit. For instance :

class MLPars(list): pass
# ...
problem_params['nvars'] = MLPars(16)
# or for 3D :
problem_params['nvars'] = MLPars([16,16,16])  # or MLPars((16,16,16))
# ...
controler = Controler_NonMPI(problem_params, ...)

And then the Controler class would just check if it receive some MLPars class as argument to trigger the multilevel instantiation. This would be safer for the implementation, and also more explicit and less error prone for the user.

[pancetta]

I see, passing lists in parameters is the pySDC way to instantiate multiple level by the controller (just remembering now 😅). That is nice indeed, but currently the constructor of some problems may bring some confusion. For instance, if we use the advectionNd (cf #244), we can instantiate the controller using the following problem_params dictionaries :

problem_params['nvars'] = [16, 16, 16]  # => MLSDC with the same space resolution on each level
problem_params['nvars'] = (16, 16, 16)  # => SDC for a 3D problem

I do feel weird about this, not sure what would be best (keeping this or trying to find an other way ... 😕)

Well, that should not be. Doing MLSDC with 3D should read

problem_params['nvars'] = [(16, 16, 16), (16, 16, 16), (16, 16, 16)]  # => MLSDC with the same space resolution on each level

[pancetta]

PS : actually, one idea to explicitly ask for multiple levels would be to use a dedicated class inherited from list (or tuple if we want to freeze it), that is passed as controller argument to be more explicit. For instance :

class MLPars(list): pass
# ...
problem_params['nvars'] = MLPars(16)
# or for 3D :
problem_params['nvars'] = MLPars([16,16,16])  # or MLPars((16,16,16))
# ...
controler = Controler_NonMPI(problem_params, ...)

And then the Controler class would just check if it receive some MLPars class as argument to trigger the multilevel instantiation. This would be safer for the implementation, and also more explicit and less error prone for the user.

Not a big fan.. I appreciate the explicitness of this, but simply following the pySDC convention of using lists for multi-level is easier and does not require yet another data structure.

[tlunet]

Discussion follows in #248