Math library input checks

By other here do you mean ones that are not at the top normal_id_glm level?

I know I am a monorepo lover and hater of submodules but I might like to see this in a separate repo that includes Stan Math as a submodule? Just so it’s easy to run the same performance test on multiple versions of the Stan Math code. When I was doing some of the other performance tests I originally had them in the same repo but it became too difficult to manage testing different commits if the test itself could change between commits. I might add a Google Benchmark-ified version to GitHub - stan-dev/perf-math if that’s okay? I might also try with the Chandler tricks above instead of the volatile and see if that makes a difference.

I mean those at top of normal_id_glm()… I just commented out the checks.

That completely makes sense. Or… having a script accept a hash?

Add to the stan-dev repo? I’d be happy with it, but we’d want to check with the TWG first. An alternative place to put it would be under the gelman · GitHub org.

Cool idea. That’s only order N and the NaN and infinite values are guaranteed to propagate.

Another idea would be to get rid of the checks on the data matrix. There’s nothing wrong in theory about letting NaN propagate. It’s what addition and multiplication do.

I’ve found the same thing whenever I used a profiler. Otherwise, I found microbenchmarks that make sure not to throw away the results were reliable.

I really liked Agner Fogg’s tutorials on C++ optimization. But it’s not about showing you how to run a profiler.

A different idea came up on the PR that should be repeated here.

We can change the preconditions for our functions in the math library. Right now, we assume that anything that is of the valid C++ type can be passed into each of our functions. We can change it so that the functions assume that the checks have already been validated.

In order to pull this off, I think we’d need to do a few things:

  1. Increment the Math library version by a major number
  2. For each function we do this with, create a separate check_* function. For example, normal_lpdf(double, double, double) should have a matching check_normal_lpdf(double, double, double) function. (naming subject to change)
  3. The Stan generator can then generate two function calls instead of one to maintain the existing behavior. Having the two functions makes it really easy to just reuse the same arguments.

If we did this, we could really start working on how to avoid generating checks and it’ll be decoupled from the function call itself. One of the difficulties will be how we handle expressions that are passed into the functions now. I’d think we’d want to store the expressions as temporaries before passing it into both functions, but I’m sure that can be worked out.

Thoughts?

I had a different implementation in mind when I proposed separating the preconditions out - I think we’d want to declaratively (rather than imperatively with a check_* function) state what the conditions are for each argument - something like this:

typedef void check_fn_ptr(double *, size_t);
std::map<const char * name, std::vector<check_fn_ptr>> function_preconditions;

So that’s a map from function name to a vector of check fns where the index lines up with the arguments to the function in question. This is assuming all check functions could be made to have the signature above, but you get the idea. Then stanc uses this function_preconditions map to generate the appropriate checks at the appropriate times.

If we wanted to provide support to Stan Math library users who aren’t using it through stanc, we can provide a templated function that calls the appropriate checks (and possibly even a version that automatically calls the checks and the actual function together).

2 Likes

That’ll work!

Map is super slow to use in a low-level algorithm like this.

I don’t see what signature the check functions could have—they’d have to take in all the arguments—it’d have to be something like parameter packs or tuples.

We could do what @syclik is suggesting without incrementing a major version number by refactoring as:

  1. T0 foo(T1 x, T2 y) { foo_check(x, y); return foo_checkless(x, y); }

  2. inline void foo_check(T1 x, T2 y);

  3. inline T0 foo_checkless(T1 x, T2 y);

Then we could expose both foo and foo_checkless. This is the usual way to factor a function so that it can be used externally through foo and internally through foo_checkless.

That way we don’t have to cobble together hacks inside the generator. It’s so much easier to develop and test in the math library than in generated code.

My fear, of course, is that users will start preferring the checkless versions of functions.

Maybe we should talk about this on a hangout? I think there’s a misunderstanding about what I wrote above… Not sure where to start but here are some things:

  1. Each check function currently applies to one datastructure and would continue to do so. The check functions would likely not change. We would arrange to call them on the appropriate arguments at the compiler level, as that is the only level that has all of the global precondition information for a Stan program. For example, if we run a bunch of functions on X that all require X to be finite, the compiler can generate check_finite(X); in the ctor of the model and never again.
  2. Confused about the hack characterization - I think we want to put much more of this kind of thing in the compiler as that’s the level at which it has global Stan information required for these optimizations (though we can keep track of some of them at runtime at additional cost). I was thinking this would wait for the rewrite, though. What’s the concern around compiler optimizations?
  3. The fact that Maps are slow seems irrelevant, as 1) it’s used just briefly by stanc and 2) we could clearly use some other type like std::vector if we realized that performance was important and we didn’t want to pay for fast associative lookups.
  4. I wouldn’t think we’d expose checkless versions in the Stan language, and Math library direct users are pretty much on their own if they did such a thing, right?

If the map you’re proposing is in stanc, then I definitely missed the point. I thought it was dynamic figuring out of when to check.

I don’t see how to do this in a clean way other than by linking it to data or transformed data blocks in the language—hence the hack characterization.

So I’m probably missing something and we should talk in person now that I’m back.

The data structure that associates checks with arguments to functions exist somewhere statically, probably in the Math library. The dynamic thing is something we could really only do with an interpreter, I think.

I have become somewhat convinced that the best way to do this is in the new stanc, along with a bunch of other stuff. I’m hoping to have a roadmap, skeleton structure, and project announcement detailing why a new stanc is important and asking for help in the next few weeks, if I can cut back on some of these meetings!

Is it okay to bring up old threads? Twice now I’ve been doing things and the pre-checks on the data came to my mind as ‘very cool and would be nice to have’.

@seantalts besides the check_fn_ptr have you ever had time to revisit this or writeup a more formal technical spec? I figure how we do this will also in part define how we do cache stuff for the GPU as well.

Maybe we could bring this up on Thursday?

1 Like

My current plan here is to get a dump of the checks every Math function requires and incorporate those into the stanc3 compiler, and then generate C++ for those checks and replaces the ones inside the Math library functions with empty functions. The Math library isn’t proving able to adapt to the lifecycle concepts we need to think about HMC iterations and this kind of thing can be optimized by the stanc3 compiler in a way the C++ compiler clearly doesn’t anyway.

2 Likes

One thing we could do is to actually have a different type for a GPU matrix. On construction, we can validate the input and then every check_* would be a no-op.

Cool! I’d go one step further and suggest that we remove the checks from Math. It would be amazing if we could write the functions with real preconditions that the client of the Math library has to adhere to. For convenience, we could have validation functions, but it’d be a bit of work… maybe something that’s scriptable.

What does that mean? I don’t understand “lifestyle concepts” (google didn’t show anything relevant) or how this links to HMC iterations (any more than optimization, etc.).

Not all matrix functions require the same conditions of their matrices; e.g. positive definite-ness.

Just to clarify a little further, I was thinking that basically when we go to compile a model, we also compile in a special Stan-level header that redefines all the Math library check_ functions to do nothing. And then yeah, what the Math library does at that point is up to the Math library, but I don’t think this feature relies on that changing (in case other users want the checks still, or whatever).

It was “lifecycle” in my post :P Just stuff like init(), tearDown, or there are some users like INLA who want to save some information between leapfrog iterations or during an entire HMC run, and none of the broader Stan lifecycles (or whatever you want to call these timelines with a start and finish) have any representation in Math right now.