Variadic is_nan function

[andrjohns]

Description

Several functions call is_nan multiple times using the || operator, such as in prim/scal/fun/grad_reg_inc_gamma:

  if (is_nan(a) || is_nan(z) || is_nan(g) || is_nan(dig))
    return std::numeric_limits<TP>::quiet_NaN();

This could be replaced with the introduction of a variadic version of is_nan (like that used for size_zero):

  if (is_nan(a, z, g, dig))
    return std::numeric_limits<TP>::quiet_NaN();

Current Version:

v2.19.1

[syclik]

This would be great for clarity and usability! Thanks!

…

On Sun, May 12, 2019 at 7:48 AM Andrew Johnson ***@***.***> wrote: Description Several functions call is_nan multiple times using the || operator, such as in prim/scal/fun/grad_reg_inc_gamma: if (is_nan(a) || is_nan(z) || is_nan(g) || is_nan(dig)) return std::numeric_limits<TP>::quiet_NaN(); This could be replaced with the introduction of a variadic version of is_nan (like that used for size_zero): if (is_nan(a, z, g, dig)) return std::numeric_limits<TP>::quiet_NaN(); Current Version: v2.19.1 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1232>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADH6F4GL7QQLZOLLYDH3ALPU77ZZANCNFSM4HMKF44A> .

[bob-carpenter]

I don't find this clear without changing the name to is_any_nan or is_nan_any to make it clear it's an OR not an AND putting the multiple arguments together.

[wds15]

I agree with bob here...i also do not disagree with doing this, but aren´t there better ways to spend our resources? Getting eigen plugins to work, for example... i am just raising the usual roi question...

[andrjohns]

I don't find this clear without changing the name to is_any_nan or is_nan_any to make it clear it's an OR not an AND putting the multiple arguments together.

I see what you mean. That then raises the question of whether we have:

• Two versions of is_nan: single inputs call is_nan(x) and multiple inputs call is_nan_any(x,y)
• One version and update the naming for all functions : single inputs call is_nan_any(x) and multiple inputs call is_nan_any(x,y)

@wds15 The Eigen plugin PR is waiting on a PR in stan, so there's not much more I can do for that atm

[betanalpha]

Opinion: I think that “is_any_nan” sounds much nice grammatically. Won’t variadic implementations just dispatch single-argument calls to the base case? In which case we could have a single function is_any_nan that holds for any size, no?

…

On May 14, 2019, at 2:14 PM, Andrew Johnson ***@***.***> wrote: I don't find this clear without changing the name to is_any_nan or is_nan_any to make it clear it's an OR not an AND putting the multiple arguments together. I see what you mean. That then raises the question of whether we have: Two versions of is_nan: single inputs call is_nan(x) and multiple inputs call is_nan_any(x,y) One version and update the naming for all functions : single inputs call is_nan_any(x) and multiple inputs call is_nan_any(x,y) @wds15 <https://github.com/wds15> The Eigen plugin PR is waiting on a PR in stan <stan-dev/stan#2754>, so there's not much more I can do for that atm — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1232?email_source=notifications&email_token=AALU3FQGOMAAUZTPVWRVVD3PVJRFDA5CNFSM4HMKF44KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVKPWRA#issuecomment-492108612>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALU3FWEDSQIUFBSTASCJLTPVJRFDANCNFSM4HMKF44A>.

[andrjohns]

Won’t variadic implementations just dispatch single-argument calls to the base case? In which case we could have a single function is_any_nan that holds for any size, no?

Yep, that's what I was suggesting as the second option above

[betanalpha]

My bad —for some reason I mistakenly thought you were suggesting an overload for the multiple-argument and single-argument. Poor reading comprehension on my part. I definitely support a single variadic “is_any_nan” implementation which has the smallest maintenance burden and most clear naming.

…

On May 14, 2019, at 6:59 PM, Andrew Johnson ***@***.***> wrote: Won’t variadic implementations just dispatch single-argument calls to the base case? In which case we could have a single function is_any_nan that holds for any size, no? Yep, that's what I was suggesting as the second option above — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1232?email_source=notifications&email_token=AALU3FQUFGEDSGKWZKQEZT3PVKSTXA5CNFSM4HMKF44KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVLH3SQ#issuecomment-492207562>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALU3FVPOEA5DZNDCJTD5RLPVKSTXANCNFSM4HMKF44A>.

[andrjohns]

No worries, equally poor clarity on my part!

@syclik and @bob-carpenter - how does that sound? A single variadic is_any_nan function?

[syclik]

I think it makes sense to keep the single arg is_nan function so we can expose that to the Stan language easily. The code gen is simple if the names match up. Does that make sense? (I’m suggesting single arg is_nan and a variadic arg is_any_nan)

…

On Wed, May 15, 2019 at 1:53 AM Andrew Johnson ***@***.***> wrote: No worries, equally poor clarity on my part! @syclik <https://github.com/syclik> and @bob-carpenter <https://github.com/bob-carpenter> - how does that sound? A single variadic is_any_nan function? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1232?email_source=notifications&email_token=AADH6F3ZND4WOXHJFCVJVUDPVOQOXA5CNFSM4HMKF44KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVNSXGQ#issuecomment-492514202>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADH6F2FLDMC56QKFZSOMIDPVOQOXANCNFSM4HMKF44A> .

[syclik]

If the template deduction makes that a pain, just go with is_any_nan.

…

On Wed, May 15, 2019 at 8:03 AM Daniel Lee ***@***.***> wrote: I think it makes sense to keep the single arg is_nan function so we can expose that to the Stan language easily. The code gen is simple if the names match up. Does that make sense? (I’m suggesting single arg is_nan and a variadic arg is_any_nan) On Wed, May 15, 2019 at 1:53 AM Andrew Johnson ***@***.***> wrote: > No worries, equally poor clarity on my part! > > @syclik <https://github.com/syclik> and @bob-carpenter > <https://github.com/bob-carpenter> - how does that sound? A single > variadic is_any_nan function? > > — > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > <#1232?email_source=notifications&email_token=AADH6F3ZND4WOXHJFCVJVUDPVOQOXA5CNFSM4HMKF44KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVNSXGQ#issuecomment-492514202>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AADH6F2FLDMC56QKFZSOMIDPVOQOXANCNFSM4HMKF44A> > . >

[andrjohns]

I’m suggesting single arg is_nan and a variadic arg is_any_nan

That's pretty easy actually, will do