Nomad I/O Validation

[Michael Betancourt]

I have a little bit of time to play around with I/O validation in Nomad,
and I want to do it pretty differently than what is currently in Stan.
In particular, all of this business with propagating NaNs is bad news,
not to mention a giant rabbit whole of numerical implementation details.

My strategy moving forward is to throw exceptions, immediately terminating
the construction of the execution graph as soon as an ill-formed value is
encountered. This will not guarantee that all the partials are well-formed,
nor that all of the dual numbers are well-formed, but I am okay with that.

This leaves a few questions.

1) What is validated?

We can certainly check for NaNs and domain errors. But what about poles
like inv(0)? Formally we could define poles as domain errors, or we could
just ignore them and let the subsequent NaNs get caught further downstream.

2) How many exceptions?

Do we have special domain and NaN exceptions, or just one common Nomad exception
with an informative message?

3) Validate inputs, outputs, or both?

We can validate input values for NaNs and appropriate domain and assume
that the output will consequently be valid.

We can also assume proper input values and assume that improper inputs
manifest as a NaN and just validate the output of the function.

Or we could validate inputs and then add an output check. This makes tracing
errors more accurate but also requires the performance hit of checking twice
plus the developer hit of coding up two checks (although this can possibly
be mitigated with some clever wrapping).

There’s always the issue of performance. We can enable_if gratuitously, but
there’s always the problem of the if blocking inline optimizations. I think the
functions are big enough this point where that won’t be an issue, but it’s
something we should monitor. We can also add an error policy to turn the
checks off, although it’s not clear if the compilers are good enough to use
that to re-enable aggressive inlining or not.

[Bob Carpenter]

More inline.

On Aug 26, 2014, at 12:16 PM, Michael Betancourt <betan...@gmail.com> wrote:

> I have a little bit of time to play around with I/O validation in Nomad,
> and I want to do it pretty differently than what is currently in Stan.
> In particular, all of this business with propagating NaNs is bad news,
> not to mention a giant rabbit whole of numerical implementation details.

It's not just Stan. If you evaluate exp(NaN) in C++, you get NaN,
not an exception. Ditto 0.0/0.0. (I don't know what 0/0 gives you
since it returns an int and there are no NaN values for ints.)

If you have exp(NaN) throw exceptions, you'll get different behaviors
with double and autodiff objects.

> My strategy moving forward is to throw exceptions, immediately terminating
> the construction of the execution graph as soon as an ill-formed value is
> encountered.

Some people and companies (e.g., Google) have a strict
no-exceptions policy in code and libraries they use.

> This will not guarantee that all the partials are well-formed,
> nor that all of the dual numbers are well-formed, but I am okay with that.
>
> This leaves a few questions.
>
> 1) What is validated?
>
> We can certainly check for NaNs and domain errors. But what about poles
> like inv(0)? Formally we could define poles as domain errors, or we could
> just ignore them and let the subsequent NaNs get caught further downstream.

There are separate errors for this kind of thing in Boost. And even
base C++ has a few different kinds of exceptions. You
can start defining your own exception hierarchy. I only see that this
makes sense if there's conceivably something on the outside that can
invoke different recovery strategies under different types of exception.
I suppose you could argue that different types of error messages might
be keyed off different types of exceptions, too.

> 2) How many exceptions?
>
> Do we have special domain and NaN exceptions, or just one common Nomad exception
> with an informative message?

See above.

> 3) Validate inputs, outputs, or both?
>
> We can validate input values for NaNs and appropriate domain and assume
> that the output will consequently be valid.

What's a valid input if not one for which a value other than NaN arises?

And what about overflows and underflows? For example, exp(1000)
overflows to +infinity, and exp(-1000) underflows to 0. Do you want
to just return 0 and a gradient of 0? Or raise an error?

> We can also assume proper input values and assume that improper inputs
> manifest as a NaN and just validate the output of the function.
>
> Or we could validate inputs and then add an output check. This makes tracing
> errors more accurate but also requires the performance hit of checking twice
> plus the developer hit of coding up two checks (although this can possibly
> be mitigated with some clever wrapping).

Of course, for some operations, like factoring a matrix, there's
no way to validate the input is valid without running the function.

> There's always the issue of performance. We can enable_if gratuitously, but
> there's always the problem of the if blocking inline optimizations.

I don't see how enable_if helps here. If you use some kind of
branch-on-template (like we do with traits to eliminate constant terms),
then it should get compiled away.

> I think the
> functions are big enough this point where that won't be an issue, but it's
> something we should monitor.

You might want to sprinkle them with unlikely, too --- branch mispredictions
cause a big slowdown.

> We can also add an error policy to turn the
> checks off, although it's not clear if the compilers are good enough to use
> that to re-enable aggressive inlining or not.

Yes, they are.

- Bob

[Ben Goodrich]

On Tuesday, August 26, 2014 12:16:46 PM UTC-4, Michael Betancourt wrote:

3) Validate inputs, outputs, or both?

If you are still thinking to require C++11 with Nomad, then you can use these macros

http://en.cppreference.com/w/cpp/numeric/fenv/FE_exceptions

to check whether FE_INVALID has become non-zero since the last time  std::feclearexcept(FE_ALL_EXCEPT) was called.

I haven't investigated the performance hit, but if it is in the standard, presumably the hit would be less than manually checking the output for NaN values. Plus, you can check for overflow, underflow, and division by zero at the same time.

Since NaN input would result in NaN output, you could get away with not checking the input specifically for NaN, but it might be better to throw an exception that mentions where the invalid input was caught.

Ben

[Michael Betancourt]

> If you have exp(NaN) throw exceptions, you'll get different behaviors
> with double and autodiff objects.

Yes, we will. And I’m fine by that.

I don’t want to deal with the abstract minutia off ill-defined specs only to
throw an error after the expression graph has been built. We might as
well err as soon as a problem arises.

> Some people and companies (e.g., Google) have a strict
> no-exceptions policy in code and libraries they use.

Yup. I also don’t know of any other way to handle this kind of run-time
error without worrying about the propagation of NaNs. Also, no way
of identifying where the expression graph went sour.

> I suppose you could argue that different types of error messages might
> be keyed off different types of exceptions, too.

That is the motivation.

> What's a valid input if not one for which a value other than NaN arises?

Within the domain of the function.

> And what about overflows and underflows? For example, exp(1000)
> overflows to +infinity, and exp(-1000) underflows to 0. Do you want
> to just return 0 and a gradient of 0? Or raise an error?

0 + 0. I don’t find underflow to be a particularly worrisome error until
someone tries to invert, in which case I’d argue it’s more naturally a
domain error of the inversion.

> Of course, for some operations, like factoring a matrix, there's
> no way to validate the input is valid without running the function.

Not entirely true — in some cases you can check validity faster
than performing the decomposition. But in any case the focus
is on scalar functions here.

> I don't see how enable_if helps here. If you use some kind of
> branch-on-template (like we do with traits to eliminate constant terms),
> then it should get compiled away.

Ugh, I meant builtin-expect. Again, the big issue is whether or not
the addition of a check, even with builtin-expect, will prevent the
compiler from inlining a bunch of code and causing noticeable
slowdowns. This was a big issue with the stack expansion
checks. I think the functions are big enough that they’re
not being inlined anywhere, but it’s hard to know a priori.

Biggest question is still unanswered — validate inputs only (requires
very strict checks and worry about things like overflow) or just have
the inputs validated for NaNs and domain errors and check again
for any divergences in the outputs?

[Bob Carpenter]

On Aug 26, 2014, at 5:39 PM, Michael Betancourt <betan...@gmail.com> wrote:

>> If you have exp(NaN) throw exceptions, you'll get different behaviors
>> with double and autodiff objects.
>
> Yes, we will. And I'm fine by that.
>
> I don't want to deal with the abstract minutia off ill-defined specs only to
> throw an error after the expression graph has been built. We might as
> well err as soon as a problem arises.

The C++ spec for exp isn't ill defined! It's clearly defined to
return NaN on NaN input.

The main problem in not following the spec is that it means people
can have code that runs with doubles and produces reasonable outputs
but can't run with Nomad. For instance, it might execute two
statements and if statement a produces NaN use statement b. This is
fine and fine for auto-diff, just not an optimal way to code things.

>> Some people and companies (e.g., Google) have a strict
>> no-exceptions policy in code and libraries they use.
>
> Yup. I also don't know of any other way to handle this kind of run-time
> error without worrying about the propagation of NaNs. Also, no way
> of identifying where the expression graph went sour.

I agree. We originally went down the route in Stan of having everything
use Boost's traits for errors, so that we could get either behavior (return
NaN or raise exception) depending on situation. Finally we decided to
drop potential users of the library and specialize to just exceptions.
I'm still of two minds about all this. It's just so damn complex with the
traits-based error handling, especially before C++11.

>> I suppose you could argue that different types of error messages might
>> be keyed off different types of exceptions, too.
>
> That is the motivation.

Usually the what() message indicates what the exception is.

>> What's a valid input if not one for which a value other than NaN arises?
>
> Within the domain of the function.

I see --- this is the same beef as with the doc Daniel's suggesting.

There are two domains in play. For exp(x), the mathematical function
is defined for x in (-inf, inf) and the output is in (0,inf). The
floating-point implementation is defined for all the floating point
numbers, including NaN and +/- inf. For +inf it returns +inf,
for -inf it returns 0 and for NaN it returns NaN. Furthermore, for
values outside of roughly (-800,800) it also returns NaN.

>> And what about overflows and underflows? For example, exp(1000)
>> overflows to +infinity, and exp(-1000) underflows to 0. Do you want
>> to just return 0 and a gradient of 0? Or raise an error?
>
> 0 + 0. I don't find underflow to be a particularly worrisome error until
> someone tries to invert, in which case I'd argue it's more naturally a
> domain error of the inversion.

What's inversion? You mean log? Underflow's a problem if people naively
multiply a bunch of probabilities together. There's no relevant inversion
anywhere.

>> Of course, for some operations, like factoring a matrix, there's
>> no way to validate the input is valid without running the function.
>

> Not entirely true -- in some cases you can check validity faster

> than performing the decomposition. But in any case the focus
> is on scalar functions here.

OK. But in some cases you can't. Or even if you could do it faster,
you don't want to do the fast thing to check, then call the function that
has the check built in.

>> I don't see how enable_if helps here. If you use some kind of
>> branch-on-template (like we do with traits to eliminate constant terms),
>> then it should get compiled away.
>
> Ugh, I meant builtin-expect. Again, the big issue is whether or not
> the addition of a check, even with builtin-expect, will prevent the
> compiler from inlining a bunch of code and causing noticeable
> slowdowns.

That I don't know the answer to. Most likely it would block inlining
and more importantly locality of code, because code still needs to
be generated for both branches, just in different orders.

> This was a big issue with the stack expansion
> checks. I think the functions are big enough that they're
> not being inlined anywhere, but it's hard to know a priori.

If you check (x + y) for x or y being NaN, that's a pretty big
hit relative to the evaluation cost.

> Biggest question is still unanswered -- validate inputs only (requires

> very strict checks and worry about things like overflow) or just have
> the inputs validated for NaNs and domain errors and check again
> for any divergences in the outputs?

I'd say check up front when you can and when it's not too computationally
expensive. That gives you finer granularity on reporting what went wrong.
But if it's too expensive, then it's probably not worth the finer grain
of reporting.

- Bob

[Michael Betancourt]

> The C++ spec for exp isn't ill defined! It's clearly defined to
> return NaN on NaN input.

But the spec is defined on a function-by-function basis. That’s what I mean.

> The main problem in not following the spec is that it means people
> can have code that runs with doubles and produces reasonable outputs
> but can't run with Nomad. For instance, it might execute two
> statements and if statement a produces NaN use statement b. This is
> fine and fine for auto-diff, just not an optimal way to code things.

No it’s not — it’s a discontinuous function waiting to happen. Again, I’m
happy making Nomad more strict to ensure that it’s also more robust.

> I see --- this is the same beef as with the doc Daniel's suggesting.
>
> There are two domains in play. For exp(x), the mathematical function
> is defined for x in (-inf, inf) and the output is in (0,inf). The
> floating-point implementation is defined for all the floating point
> numbers, including NaN and +/- inf. For +inf it returns +inf,
> for -inf it returns 0 and for NaN it returns NaN. Furthermore, for
> values outside of roughly (-800,800) it also returns NaN.

Checking the “empirical” domain would be a pain in the ass and cause
no end of developer problems. Hence the option to add an output check
(or more precisely, checks when trying to push values and partials onto
the respective stacks).

> What's inversion? You mean log? Underflow's a problem if people naively
> multiply a bunch of probabilities together. There's no relevant inversion
> anywhere.

A probability density smaller than the smallest floating point number being represented
by zero is fine. You only run into issues when you need a nonzero probability density, like
trying to compute the log density...

> If you check (x + y) for x or y being NaN, that's a pretty big
> hit relative to the evaluation cost.
>
>> Biggest question is still unanswered -- validate inputs only (requires
>> very strict checks and worry about things like overflow) or just have
>> the inputs validated for NaNs and domain errors and check again
>> for any divergences in the outputs?
>
> I'd say check up front when you can and when it's not too computationally
> expensive. That gives you finer granularity on reporting what went wrong.
> But if it's too expensive, then it's probably not worth the finer grain
> of reporting.

But we’re not just computing x + y — we’re also creating a node in the expression graph
and incrementing the stacks (which have if statements that already mess up aggressive
inlining). All of the autodiff overhead would nominally make the check overhead
negligible.

I’m also not going to implement bespoke checks for every function separately. That’s
too much of a pain for development and testing. If we want a robust, readily debuggable
stack then we’re going to have to take the performance hit.

Certainly no one is monitoring Stan’s performance as these kinds of checks are
being added.

[Bob Carpenter]

On Aug 26, 2014, at 6:16 PM, Michael Betancourt <betan...@gmail.com> wrote:

>> The C++ spec for exp isn't ill defined! It's clearly defined to
>> return NaN on NaN input.
>
>
> But the spec is defined on a function-by-function basis. That's what I mean.
>

The core C++ library functions all do the same thing. They
all return NaN when given NaN inputs. And they raise error
flags. For instance, if exp() overflows, it sets errno to
ERANGE, indicating a range problem (i.e., value can't be
represented in floating point).

>> The main problem in not following the spec is that it means people
>> can have code that runs with doubles and produces reasonable outputs
>> but can't run with Nomad. For instance, it might execute two
>> statements and if statement a produces NaN use statement b. This is
>> fine and fine for auto-diff, just not an optimal way to code things.
>

> No it's not -- it's a discontinuous function waiting to happen. Again, I'm

> happy making Nomad more strict to ensure that it's also more robust.

Two different kinds of robustness. Robustness to bad derivatives
vs. robustness to bad code. Both sides have something to say for them,
and I'm OK either way.

>
>> I see --- this is the same beef as with the doc Daniel's suggesting.
>>
>> There are two domains in play. For exp(x), the mathematical function
>> is defined for x in (-inf, inf) and the output is in (0,inf). The
>> floating-point implementation is defined for all the floating point
>> numbers, including NaN and +/- inf. For +inf it returns +inf,
>> for -inf it returns 0 and for NaN it returns NaN. Furthermore, for
>> values outside of roughly (-800,800) it also returns NaN.
>
> Checking the "empirical" domain would be a pain in the ass and cause
> no end of developer problems. Hence the option to add an output check
> (or more precisely, checks when trying to push values and partials onto
> the respective stacks).

Agreed.

>> What's inversion? You mean log? Underflow's a problem if people naively
>> multiply a bunch of probabilities together. There's no relevant inversion
>> anywhere.
>
> A probability density smaller than the smallest floating point number being represented
> by zero is fine. You only run into issues when you need a nonzero probability density, like
> trying to compute the log density...
>
>> If you check (x + y) for x or y being NaN, that's a pretty big
>> hit relative to the evaluation cost.
>>
>>> Biggest question is still unanswered -- validate inputs only (requires
>>> very strict checks and worry about things like overflow) or just have
>>> the inputs validated for NaNs and domain errors and check again
>>> for any divergences in the outputs?
>>
>> I'd say check up front when you can and when it's not too computationally
>> expensive. That gives you finer granularity on reporting what went wrong.
>> But if it's too expensive, then it's probably not worth the finer grain
>> of reporting.
>

> But we're not just computing x + y -- we're also creating a node in the expression graph

> and incrementing the stacks (which have if statements that already mess up aggressive
> inlining). All of the autodiff overhead would nominally make the check overhead
> negligible.

Maybe. Only way you'll know is to profile it.

Are if statements particularly bad for inlining? I was thinking
of them as more problematic for prefetching instructions and
branch prediction.

> I'm also not going to implement bespoke checks for every function separately. That's
> too much of a pain for development and testing. If we want a robust, readily debuggable
> stack then we're going to have to take the performance hit.

How are you going to do that? Just check the nodes getting created
on the autodiff stack? Will you be able to have messages saying which
function the error came from?

> Certainly no one is monitoring Stan's performance as these kinds of checks are
> being added.

You're right. But we should be monitoring this kind of thing,
at least with microbenchmarks.

- Bob

[Michael Betancourt]

Okay, let me step back a second.  What is the point of input/output validation?

C++ handles domain errors by outputting a NaN and letting that propagate through the expression graph.

This would be by far the simplest and most computationally efficient strategy, with our checking the

output value at the end to see if the results are valid.

Stan currently has exceptions for domain errors (at least the probability distributions do).  This makes it

easy to identify where a problem occurs.  Checking for NaNs at input and at output and throwing exceptions

also helps with identifying problems.  The issue with all of these checks and throws is that the code

becomes a lot uglier and significantly slower.  Adding template parameters to turn off the checks makes

the code even uglier.

So before moving forward beyond the simple implementation that I have, is there any reason to do these

checks other than to identify where an issue occurs?  And is it really that useful to know where a NaN

first appears (essentially trying to identify where overflows first become an issue)?

And, ultimately, is that info sufficiently helpful that it's worth making functions harder to implement?

- Bob

--
You received this message because you are subscribed to the Google Groups "stan development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Bob Carpenter]

I think that's exactly the right framing of the question.

The one additional thing I'd add is fidelity of the autodiff
arithmetic to the double-based arithmetic. Again, I'm not sure
how critical that is. For instance, is it OK to use two different
exp() implementations?

If we wanted to make our auto-diff library most useful,
we'd have configurable behavior. This is what both Eigen
and Boost do. It does add a bit of complexity to use,
with Eigen's environment variable being perhaps a little
coarser but easier to use than Boost's template traits.

I also agree that programming time, and by that I include doc,
testing and maintenance is a relevant concern. We want to
balance building the world's best autodiff library versus
actually delivering one in a finite amount of time.

C++ also sets a state-monitoring flag when arithmetic
goes bad, but it's up to the application to decide what to
do with it.

- Bob

[Marcus Brubaker]

If memory serves, the issue that sparked the recent obsession with NaN was that a function was producing non-NaN outputs for a NaN input due to some special casing.  This is clearly bad behaviour and should be tested for and prevented.  I would err on the level of following C++/IEEE floating-point and just allowing NaN to propagate up and allowing a user (meaning C++ API user) to check for NaNs when/if they want rather than including explicit isnan() checks all over the place which will hurt performance (potentially quite badly) under normal, non-NaN operation.  Given that, combined with the added programming overhead that adding all of these checks (and templating them out) would add, I'd say the policy should be to allow NaNs to propagate and leave it to the user to check.

For Stan we want to catch NaNs as early as is practical to reject bad samples.  There are two (main) reasons to do this:

1. For speed, so we can stop computing the rest of the posterior function when we know it's going to be thrown out.

2. For ease of debugging, so that we can report to the user more information about where the NaN came from and why the sample was rejected.

Adding a large number of isnan() checks which slow down normal operation simply to get a small gain when we (hopefully rarely) reject a sample early seems like a loosing proposition.  Ease of debugging is a nice reason, but if we catch NaNs when, e.g., they're added to the log prob accumulator or when a distribution is computed, I think we'd still have a reasonable level of debugging information with less performance overhead.

Cheers,

Marcus

[Ben Goodrich]

On Friday, August 29, 2014 12:55:59 PM UTC-4, Bob Carpenter wrote:

C++ also sets a state-monitoring flag when arithmetic
goes bad, but it's up to the application to decide what to
do with it.

But that behavior was only standardized in the C++11 standard. I am not sure what the current mingw in Rtools would do with the fpenv stuff (or the older g++ versions).

Ben

[Daniel Lee]

On Fri, Aug 29, 2014 at 10:03 AM, Michael Betancourt <betan...@gmail.com> wrote:

Okay, let me step back a second.  What is the point of input/output validation?

I'll second Bob's sentiment: thanks for taking a step back.

The point of input/output validation is to implement a consistent specification of our functions. In all our discussions, I'm actually surprised an alternative proposal hasn't come up: specifying preconditions. That would put the burden on the caller. I think the reason we're doing all this validation now is because we allow any double to get in and we don't want undefined behavior of our functions which would mask errors. by that, I mean generating and calling the log probability function which should have resulted in an error, but doesn't.

I'll try to outline different options: suppose we have two functions, foo(double x) and bar(double y). Let's assume we're trying to do bar(foo(x)), for some double value, x. Under all cases, bar(foo(x)) evaluates to the right thing where it's well behaved.

1. If we always checked input:

  - bar(foo(NaN)) would stop and we'd know that foo was called improperly

  - bar(foo(x)), where foo(x) evaluates to NaN, would stop and we'd know that bar was called improperly, but we wouldn't know that foo(x) wasn't run properly. Of course, if we always checked input, we shouldn't be returning NaN ever, so this case wouldn't exist.

2. If we always checked output:

  - bar(foo(NaN)) where foo(NaN) returns a NaN will report an error stating that foo didn't finish properly.
  - bar(foo(NaN)) where foo(NaN) returns a normal number will not report an error. This is where we could be in a lot of trouble.

3. If we had a bunch of preconditions, we'd need to do something like:

  - bar(foo(x)) can't be evaluated directly. We would have to do: check(x); foo(x); check(foo(x)); bar(foo(x)); But we'd have the same behavior as 1.

I think what we're doing now is trying to tighten up the spec by stating that if a function argument takes in an NaN, it must return NaN (where sensible) or it must throw an exception. It is not allowed to take NaN and then return some number like 0 or 1. This is happening now in Stan code.

I wanted to respond to Marcus's point.

On Fri, Aug 29, 2014 at 2:11 PM, Marcus Brubaker <marcus.a...@gmail.com> wrote:

If memory serves, the issue that sparked the recent obsession with NaN was that a function was producing non-NaN outputs for a NaN input due to some special casing.

No, it wasn't due to special casing. Most of our issues have to do with inconsistent implementations by Boost and Eigen where an NaN input produces non-NaN output when the standard library does produce NaN.

This is clearly bad behaviour and should be tested for and prevented.  I would err on the level of following C++/IEEE floating-point and just allowing NaN to propagate up and allowing a user (meaning C++ API user) to check for NaNs when/if they want...

That's pretty much what we've gone with.

 

rather than including explicit isnan() checks all over the place which will hurt performance (potentially quite badly) under normal, non-NaN operation.  

I haven't seen the isnan() check actually hurt performance by anything measurable on my machine using clang++ and O=3. Can you show where it does hurt performance and if that's the case, we can really think about how to get the flexibility in there? I'm more concerned with dev time over absolute breakneck speed because if / when this bug gets touched on, it'll take us a lot of time to hunt down.

 

Given that, combined with the added programming overhead that adding all of these checks (and templating them out) would add, I'd say the policy should be to allow NaNs to propagate and leave it to the user to check.

There are two issues here:

1. Without checking, our implementation would mask input NaN and return a number (usually 0 or 1... that's based on just seeing what Boost and Eigen do with these functions).

2. Sounds like a redesign of our error handling. I'm happy to do it, but I think we need a design and some principled way to do this. I'm thinking that some checks should always be run and some should be optional. Optional ones include bounds checks on matrices and looks like isnan() except where necessary to follow spec. I'm all for it if someone comes up with a reasonable design.

Adding a large number of isnan() checks which slow down normal operation simply to get a small gain when we (hopefully rarely) reject a sample early seems like a loosing proposition.  

I'd like to see some (any) numbers that shows how much it slows down normal operation. I've done simple timings where I've commented out the checks and run the executable without seeing any difference in time over hundreds of runs. But if you've got a concrete example, then I'll take it into consideration.

 

Ease of debugging is a nice reason, but if we catch NaNs when, e.g., they're added to the log prob accumulator or when a distribution is computed, I think we'd still have a reasonable level of debugging information with less performance overhead.

This is assuming that functions properly propagate NaN without checking for NaN, which is not the case. We need the check in order to propagate NaN. So under this condition, I think it's appropriate to leave it in and either return NaN or throw (depending on the function).

[Michael Betancourt]

To set the stage a bit — right now Nomad uses only the standard library or custom implementations,
so there’s no immediate concern about Boost and Eigen behavior (that said, it’s easy to wrap calls
to those libraries around NaN checks later). Consequently, the NaN behavior in Nomad is consistent
and it’s a question of whether we want to isolate the source of the problem for the user. Details
such as how the checks are done (isnan vs. checking error flags, checking inputs and/or outputs)
are second-order.

These source messages have historically been useful for both users and our own ability to debug
users’ code so I understand why we would want to keep that functionality. The compromise is
either speed (add input and output checks in Nomad results in 5-25% slowdowns depending on
the order of the method, etc) or code complication. For example, this is what sort would look like,

template <short autodiff_order, bool strict_smoothness>
inline var<autodiff_order, strict_smoothness>
sqrt(const var<autodiff_order, strict_smoothness>& input) {

double input_val = input.first_val();
if (unlikely(std::isnan(input_val))) throw nomad_input_error("sqrt");
if (unlikely(input_val < 0)) throw nomad_domain_error("sqrt");

const short partials_order = 3;
const unsigned int n_inputs = 1;

next_inputs_delta = n_inputs;
next_partials_delta =
unary_var_body<autodiff_order, partials_order>::n_partials();

new unary_var_body<autodiff_order, partials_order>();

double val = std::sqrt(input.first_val());

try {
push_dual_numbers<autodiff_order>(val);
} catch(nomad_error& e) {
throw nomad_output_error("sqrt");
}

push_inputs(input.dual_numbers());

double d2 = 1.0 / input.first_val();

try {
if (autodiff_order >= 1) push_partials(val *= 0.5 * d2);
if (autodiff_order >= 2) push_partials(val *= - 0.5 * d2);
if (autodiff_order >= 3) push_partials(val *= - 1.5 * d2);
} catch(nomad_error& e) {
throw nomad_output_error("sqrt");
}

return var<autodiff_order, strict_smoothness>(next_body_idx_ - 1);

}

and this is before adding a third template parameter to var to toggle the checks and
restore top speed.

It’s no problem for me to go through and add all of this, it will just make adding new
functions more of a pain for those who are not familiar with the architecture.

Another possibility brought by Marcus is to make the source identification a feature
of Stan and not Nomad by having the internal accumulator created by the parser
do checks. Something of a compromise.

So what do people think? Fast code with a “debug” mode that finds the source of
problems at the expense of denser code or another compromise?

[Marcus Brubaker]

So my concern with the speed hit of isnan() checks is based on profiling I did a while back where even simpler things like bound and size checks started popping up (~10%) in profiles on large models (a multivariate, latent factor stochastic volatility time series model with ~12 variates, ~3 factors and ~3000 timesteps).  If you're not seeing it, then maybe it's not a big deal, but it feels like we've made a number of design decisions lately which have trimmed performance in hopefully marginal ways but we don't have a good way of really measuring their impact in any systematic way.  That makes me nervous.

I agree that the policy should be that we require (and unit test) that all numeric functions propagate NaN through: NaN in means NaN out.  I want to applaud you and Peter for the work on going through and checking this.  For most functions this is the C++/IEEE defined behaviour so shouldn't require any explicit isnan() checks, so there shouldn't be any performance hit.  More complex functions (e.g., boost special functions) may require an explicit isnan() check, but in that case it seems unlikely to hurt.  Then, in generated models, we check for NaN where convenient and reasonably efficient based on our ability to accurately report that information to the user.

Where we may be differing is how often those NaN checks should be performed.  There is a spectrum here from checking before or after every single function call to only checking the final log_prob value.  Given that there are sometimes legitimate uses for allowing a NaN through (e.g., when using an approximation for parts of a numerically unstable function or when performing an operation on a matrix where only some values will actually be used) I would argue that checking every single function call is excessive.  Beyond that though I think there are any number of reasonable options for when and how to check for NaNs.

Cheers,

Marcus

[Marcus Brubaker]

As a side note, as far as I know likely()/unlikely() is a gcc specific extension (which clang supports).  I don't think MSVC supports it though, so making it an integral part of efficient operation may hurt portability a bit.  Not sure how much anyone really cares about MSVC though....

Cheers,

Marcus

On Fri, Aug 29, 2014 at 4:02 PM, Michael Betancourt <betan...@gmail.com> wrote:

To set the stage a bit -- right now Nomad uses only the standard library or custom implementations,

[Bob Carpenter]

There's also some confusion here in that there's two systems
under discussion, the existing autodiff and the new "nomad"
autodiff.

My only concern is ensuring the behavior on var matches that
of double.

Given the serious issue with speed, there should be a way to
turn any additional checks off. I'm OK doing that with either
templates or environment variables --- the latter are a bit
easier conceptually on users, but they're already buying into
a lot of template complexity and a bit more is probably no big
deal. So to that end whatever's easier to test and maintain seems
like a good idea.

An example of where we had problems was with using Eigen's
elementwise .exp() --- it didn't comply with the std C++
lib's behavior. Now Boost is using their own specialized
exp() implementation whose name suggests it's tuned for
SSE instructions on chips and hence faster than the std version.
That's why I brought up how much compliance with the std lib
we want.

I don't think the flags are new to C++11. If you look here:

http://www.cplusplus.com/reference/cmath/exp/?kw=exp

you'll see that the earlier version of C++ also sets flags.

So the issue for compatibility is whether we should also be calling
the std::exp to make sure we set the flags the right way.

Yet another problem is that there are multiple versions of these
things built into C++, some coming in through the C lib in cmath,
some coming with no namespace, and some coming with regular includes.

I think we also had Boost's lgamma throwing
exceptions where the std lib version didn't. I don't think
we need the templated one from Boost any more.

- Bob

[Michael Betancourt]

Two points:

Right now Nomad defines a double version and a var version of each
function in the nomad namespace to ensure that the function values
are guaranteed to be consistent. This pattern ensures that we can
maintain consistency even if we add Eigen/Boost functions.

The template parameters are all masked by typedefs. So we have
the default var1, var2, var3; the variants that admit non smooth
functions, var1_wild, var2_wild, var3_wild; and nominally variants
that enable all of the i/o checking, var1_debug, var2_debug, var3_debug.
Of course a power user could specify the var template parameters
explicitly, but a casual user does’t have to worry about that detail.

[Bob Carpenter]

That won't guarantee consistency with the standard library versions
of functions for which such exists. Which is why I keep trying to
bring up how important this feature is. Maybe we don't need to worry
about it --- clearly Eigen wasn't too bothered.

Are you also maintaining separate versions of the operator arithmetic,
too, like addition and multiplication?

I think the typedefs are a good idea. Having said that,
any choice a user has to make among options adds to the complexity of
both documentation and use (as well as testing, but that's not a user
problem). Complexity isn't just syntactic!

- Bob

[Michael Betancourt]

> That won't guarantee consistency with the standard library versions
> of functions for which such exists. Which is why I keep trying to
> bring up how important this feature is. Maybe we don't need to worry
> about it --- clearly Eigen wasn't too bothered.

We can guarantee consistency within nomad. If someone calls some
implementation outside of Nomad I don’t care.

> Are you also maintaining separate versions of the operator arithmetic,
> too, like addition and multiplication?

No, not for the basic operations, as those are all std lib.

> I think the typedefs are a good idea. Having said that,
> any choice a user has to make among options adds to the complexity of
> both documentation and use (as well as testing, but that's not a user
> problem). Complexity isn't just syntactic!

Yes, but I consider that a feature here. It takes some thought to use
non smooth functions or enable debug mode, etc. Forced mindfulness,
perhaps?