When to use type annotations for function parameters: best practice and what to do in Trixi?

[ranocha]

In GitLab by @sloede on May 11, 2020, 14:28

@ranocha started a discussion in #42 about the (mis-)use of type annotations for function parameters, e.g. in src/solvers/dg.jl:

function calc_surface_flux!(surface_flux::Array{Float64, 4}, neighbor_ids::Matrix{Int},
                            u_surfaces::Array{Float64, 4}, dg::Dg, ::Val{true},
                            orientations::Vector{Int})
   ...
end

He suggested to remove type annotations that purely serve documentation purposes and are not required for multiple dispatch (in the example: all but ::Val{true}), and to instead capture the documentation aspects in a docstring instead. To me, this seems like a very reasonable proposal. However, before changing anything in Trixi, it would help (at least me) to have these two questions answered first:

1. What are best practices in the Julia community regarding type annotations for function parameters?
2. What is a good approach for us in Trixi, taking into account added complexity, required effort, and what can be reasonably implemented in practice?

As far as I understand, there are only two cases where you must use type annotations: If it is required for multiple dispatch, or if you use parametric types for which you need to access/use the actual type. In all other cases I can think of, type annotations are optional and, quoting the Julia manual:

Adding annotations serves three primary purposes: to take advantage of Julia's powerful multiple-dispatch mechanism, to improve human readability, and to catch programmer errors.

I think most cases in Trixi belong to the second or third category (or a mixture thereof): We often use type annotations like the ones above to show a user/programmer what kind of function parameters are expected, and to immediately throw an error if a programmer violates this. For example, in DG code we often have methods like flux or cons2prim that could very well work on the entire DG dataset (i.e., all elements, all nodes) or just on a single point.

Thus, back to my questions: What are the best practices ("the Julian way") and how do we want handle this in Trixi? The biggest advantages of the current style (annotations-as-documentation) are IMHO that

1. They are always up to date.
2. They are always use the same "formatting".

The biggest advantages of using docstrings are IMHO

1. It does not abuse the type annotations for documentation, leaving the code more generic in the process.
2. It allows to use Julia's internal doc mechanisms on the REPL
3. Documentation is not just in the code but also on the GitLab Pages website.
   If we start to move towards using non-annotated function parameters together with docstrings, we would have to consider what our own guidelines would be, such that it is easy for students (and for ourselves) to decide when and how to put documentation in docstrings.

I have not yet made up my mind yet about the way we want to handle it in Trixi, but would be very interested in your ideas! Also, I'd by very interested in learning about the "true" Julian way - maybe @ranocha can shed some more light on this, since he is by far the most experienced Julia developer here ;-)

[ranocha]

In GitLab by @ranocha on May 11, 2020, 14:47

I'll try to answer your questions here.

1. What are best practices in the Julia community regarding type annotations for function parameters?

   My understanding is: "Use them if and only if they are necessary for dispatch". At least this is the approach that seems to be taken by the standard library modules of Julia itself and many other packages in the ecosystem I use, e.g. OrdinaryDiffEq.jl.

2. What is a good approach for us in Trixi, taking into account added complexity, required effort, and what can be reasonably implemented in practice?

   Many methods are called with arguments constructed as parts of one of the basic types such as Dg. If someone calls them with totally wrong arguments, a MethodError will probably be thrown inside of such a method since something doesn't fit. That could be enough indication (together with the stacktrace and calling signature) to find the bug.
   For example, that surface_flux::Array{T, 4} where T will become clear in the code since surface_flux is used with four indices and a method error will be thrown if someone passes a scalar as surface_flux.

Personally, I tend to like docstrings very much and would like to use more of them in Trixi. If I code and test something in the REPL, I usually try to get some answers from docstrings instead of asking Google at first. It also serves as additional documentation. For example, we could also add DOIs to docstrings of numerical fluxes etc.

[ranocha]

In GitLab by @sloede on May 20, 2020, 09:33

Summing up the conclusions of our discussion on Monday:

• From now on, we should not introduce more type annotations for documentation purposes. They should only be used to control multiple dispatch.
• Instead, if we want to document behavior, we should either use a docstring (preferred version) or a regular comment (lazy version).
• If one encounters a for-documentation-purposes-type-annotated function during development, it is encouraged to remove the type annotations and replace it with a docstring/comment.

Please fix it directly here if I did not get it right. @ranocha I think we can close this issue now. If you think it is necessary to remove all existing unnecessary type annotations, I think we should create a new issue specifically for that.

[ranocha]

In GitLab by @ranocha on May 20, 2020, 09:49

Sounds good for me.

[ranocha]

In GitLab by @sloede on May 20, 2020, 09:53

closed