Default arguments/Parameter packs

I was working on using parameter packs in the ODE solves and there seems to be a conflict between default arguments and parameter packs as they’re implemented in C++ that I guess we should talk about.

So we either do functions where we do the parameter packs first:

f(T_Args... args, double optional1 = 0.0, double optional2 = 0.0)

Or ones with the parameter packs second:

f(double optional1 = 0.0, double optional2 = 0.0, T_Args... args)

If we do the first, C++ won’t automatically deduce T_Args for us.

If we do the second, then any time we provide args, we have to provide values for all the optional args.

The first seems better since we always have type information. It’s a bit annoying though, and if nothing changed then we know we’ll have problems in the future if Stan ever supports type deduction in the language itself.

Completely wild and probably deeply stupid option (ignore at will): could the optional parameters be template arguments?

It’s tolerances for the ODE solvers so we gotta have them modifiable at runtime.

Are the options passed in functions or methods in classes? If f exists in a class you could set the optional args as a constructor parameter. T_args can’t be deduced in the first case because C++ can’t tell when a parameter pack ends and standard function parameters begin. This doesn’t solve the problem at all, but if they are optional what if you just brought in a struct of options like

struct optional_struct {
// yada yada
};

option_struct blah(the, options);
f(blah, other, stuff);

At least then it’s a simple pattern and when you don’t want any optional stuff send in an empty optional_struct

This is for the user-facing integrate_ode_* calls.

I think I’ve vaguely decided to go with the first thing.

I had the second thing coded up and it was giving me an error I didn’t believe so I just gave up. I think the first is better and we’ll just have to deal with passing around types.

It’ll just make the tests more annoying to code, but I don’t think it affects us too badly. Talking through it, the first thing seems like the only reasonable way too.

It feels like (1) is going to be way more trouble to code around. Do you have this on a branch somewhere?

If it’s user facing what if they passed them as temp array like

integrate_ode_blah({0.1, 100}, the, stuff, here);

I don’t know actually. I’m a little confused on this again.

I guess there aren’t actually optional parameters in the Stan language itself, there are just two signatures for each of the integrate_ode functions (https://mc-stan.org/docs/2_21/functions-reference/functions-ode-solver.html)

In that case I think the second thing makes more sense and we shouldn’t be taking advantage of default arguments in C++ to implement this (and instead just implement the two signatures separately).

But what I just said seems a bit shortsighted – like what if we actually get default arguments (edit: default arguments in Stan, I mean)?

Then the parser will spit out c++ function calls with the arguments set as needed, no? It’s a parser issue to deal with.

…btw, it would be cool to allow for … in the Stan Language…

I settled on passing in the types explicitly. So if the old call to integrate_ode_rk45 is:

integrate_ode_rk45(harm_osc, y0, t0, ts, theta, x, x_int)

then the new one is:

integrate_ode_rk45<std::vector<double>, std::vector<double>, std::vector<int>>(harm_osc, y0, t0, ts, theta, x, x_int)

The signature looks like:

template <typename ...Args, typename F, typename T1, typename T_t0, typename T_ts>
std::vector<std::vector<return_type_t<T1, T_t0, T_ts, Args...>>> integrate_ode_rk45(
    const F& f, const std::vector<T1>& y0, const T_t0& t0,
    const std::vector<T_ts>& ts,
    const Args&... args,
    std::ostream* msgs = nullptr,
    double relative_tolerance = 1e-6,
    double absolute_tolerance = 1e-6,
    int max_num_steps = 1e6) {

That’ll be enough to get me through this first attempt at refactor at least.

You’ll have to talk to the compiler folks to figure out how hard it would be to generate the C++ for that.

What’s wrong with forcing users to pass in a vector as the first arg? Are they parameters users change during runtime?

tbh wish we had structs in the language

Nah, they’d really only hard code these into their models or pass them in as data. They’re not allowed to be parameters anyway.

I think I’ll just work around what we’ve got for now. A problem with the vector solution is that these arguments are different types – the tolerances are doubles and the max_steps is an integer. Also these are different arguments. There should be a way to represent them as such.

Alright yeah this is a problem and this’ll need to be something figured out at the Stan language level.

If we just extend the current ODE signatures and try to make the arguments variable length we’d get:

integrator(function ode, real[] initial_state, real initial_time, real[] times,
          T1 arg1, T2 arg2, ...)
integrator(function ode, real[] initial_state, real initial_time, real[] times,
           T1 arg1, T2 arg2, ...,
           double reltol, double abstol, int maxsteps)

And ode would need the signature:

real[] ode(real time, real[] state, T1 arg1, T2 arg2, ...)

The problem is if I made the call:

integrator(my_func, y0, t0, ts, 1e-6, 1e-6, 1e6)

Are those last three arguments meant to be passed through to the ODE function as arg1, arg2, …, or do they specify the tolerances + maxsteps?

We can solve this problem in C++ by passing template arguments for what T1, T2, etc. are specifically (which is the earlier part of this thread).

(edit: the same ambiguity exists if we try to put the optional parameters before the variadic parameters)

Nope. Only integer types are allowed.

I agree with @Stevo15025 here. The traditional way to get around this is to explicitly define two signatures, where the optional args version f(T_Args...) delegates to the explicit-args version by filling in defaults and calling f(double, double, T_Args...). That’s how it’s set up in the Stan language as @bbbales2 notes:

Yes, the Stan language is the target. So we have to design down from there.

That’s coming. You want to work on that instead of sparse matrices? It’s much more manageable as a project because it doesn’t require math lib support.

This is why they have to go first—otherwise, we get ambiguity. Another way to remove the ambiguity is using a separately named function like ode_integrator_rk45_tol.

That seems like the most convenient solution in this case.