Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect use of NaN #4

Open
conradsnicta opened this issue Jan 10, 2022 · 5 comments
Open

incorrect use of NaN #4

conradsnicta opened this issue Jan 10, 2022 · 5 comments

Comments

@conradsnicta
Copy link

@simonsays1980 This bit of code in optimize.h is not likely to work correctly:

finmix/src/optimize.h

Lines 67 to 70 in d853be4

else if (rvalues.has_nan())
{
rvalues.elem(arma::find(rvalues == arma::datum::nan)).zeros();
}

Extract from cppreference.com: "NaN values never compare equal to themselves or to other NaN values".
https://en.cppreference.com/w/cpp/numeric/math/isnan

Suggest to use arma::replace() instead, in order to replace NaN values with zeros.

CC: @eddelbuettel

@eddelbuettel
Copy link

Hi guys, coming in here without too much context but the main take away is that R use the standard IEEE approach for NaN on double but then .... goes beyond by a) also defining NA and b) doing so for types besides double. Which is extremely valuable for the statistical context R is in -- but also not portable. So while it may work for NaN on double with Armadillo, it is no passthrough.

We do have documentation (a little scattered, I admit) on how to do withing the R context via Rcpp and hence RcppArmadillo.

@eddelbuettel
Copy link

To the point @conradsnicta was making, a more C++-standards compliant way is std::isfinite -- see https://en.cppreference.com/w/cpp/numeric/math/isfinite. But yes the == comparison will not work, just how it does not in R either:

> x <- c(3, NA, NaN, 5)
> x == NA
[1] NA NA NA NA
> sapply(x, is.finite)
[1]  TRUE FALSE FALSE  TRUE
> 

@simonsays1980
Copy link
Owner

simonsays1980 commented Jan 10, 2022 via email

@simonsays1980
Copy link
Owner

simonsays1980 commented Jan 10, 2022 via email

@eddelbuettel
Copy link

Hi Simon,

This hit me out of context per Conrad's tag but I think he is right on that you cannot do == comparison for non-finite value but need to use a function. And there is merit to both sides: I have code where I try keep files 'portable' without R headers and if the variable is floating point then maybe one of the STL functions can sweep a std::isfinite(rvalues) across an Armadillo vector (if you need positions). I always have to look up std::find_if and alike. Else arma::replace as Conrad suggested may do what you need.

What is nice about the Rcpp(Armadillo) integration is that you can test that oneliners immediately. And Rcpp helps with a few of the vectorised functions.

> Rcpp::cppFunction("LogicalVector foo(NumericVector x) { return is_finite(x); }")
> foo(c(2, 3, NA, 5, NaN, 7, Inf, 9))
[1]  TRUE  TRUE FALSE  TRUE FALSE  TRUE FALSE  TRUE
> 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants