[Rd] stats/ar.R issue - else branch never used
Hello, There is another issue in stats/ar.R: https://github.com/wch/r-source/blob/4a1ed749271c52e60a85e794e6f34b0831efb1ae/src/library/stats/R/ar.R#L142 https://github.com/wch/r-source/blob/4a1ed749271c52e60a85e794e6f34b0831efb1ae/src/library/stats/R/ar.R#L429 https://github.com/wch/r-source/blob/4a1ed749271c52e60a85e794e6f34b0831efb1ae/src/library/stats/R/ar.R#L570 When aic is passed as a boolean (T/F), the code does the following: maic <- min(aic) # 0 if F, 1 if T xaic <- setNames(if(is.finite(maic)) xaic - min(xaic) else ifelse(xaic == maic, 0, Inf), 0L:order.max) Because maic is always 0 or 1, is.finite(maic) is always T, so the else branch (ifelse(xaic == maic, 0, Inf)) never executes. Please check if my finding is valid. Best regards, Norbert Kuder [[alternative HTML version deleted]] __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
[Rd] stats/HoltWinters.R inverted logic in seasonal in R and C
Hello,
I have noticed a potentially confusing implementation in the HoltWinters
function regarding the seasonal parameter mapping between R and C code:
https://github.com/wch/r-source/blob/4a1ed749271c52e60a85e794e6f34b0831efb1ae/src/library/stats/R/HoltWinters.R#L98
The C code interprets a seasonal value of 1 as additive and 0 as
multiplicative.
The R seasonal can be "additive" or "multiplicative", so the R code must
invert the logic when calling C.
The proposed solution is to define a seasonalChoice variable:
hw <- function(x, alpha, beta, gamma, seasonal, start.time, f, ...) {
lenx <- length(x)
seasonalChoice <- if (seasonal == "multiplicative") 0L else 1L
.C(C_HoltWinters,
as.double(x),
lenx,
as.double(max(min(alpha, 1), 0)),
as.double(max(min(beta, 1), 0)),
as.double(max(min(gamma, 1), 0)),
as.integer(start.time),
as.integer(seasonalChoice),
as.integer(f),
as.integer(!is.logical(beta) || beta),
as.integer(!is.logical(gamma) || gamma),
...
)
}
Please check the proposed solution.
Regards,
Norbert Kuder
[[alternative HTML version deleted]]
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Possible issue in stats/arima.R package
Martin,
thank you solving this issue. Anyway there is the same bug in arima0.R:
https://github.com/wch/r-source/blob/4a1ed749271c52e60a85e794e6f34b0831efb1ae/src/library/stats/R/arma0.R#L79
Best regards,
Norbert Kuder
czw., 2 sty 2025, 21:53 użytkownik Martin Maechler <
maech...@stat.math.ethz.ch> napisał:
> >>>>> Martin Maechler on Thu, 2 Jan 2025 20:42:58 +0100 writes:
> >>>>> Duncan Murdoch on Thu, 2 Jan 2025 11:28:45 -0500 writes:
> >> On 2025-01-02 11:20 a.m., Duncan Murdoch wrote:
> >>> On 2025-01-02 9:04 a.m., Norbert Kuder wrote:
> >>>> Hello all,
> >>>>
> >>>> I am running R version 4.4.2 (2024-10-31 ucrt) on Windows 10 x64,
> and
> >>>> noticed something that might be a minor bug (or at least
> inconsistent code)
> >>>> in the stats/arima.R package.
> >>>> I have found:
> >>>> 1. A missing stop() call at line 69:
> >>>> if (length(order) == 3) seasonal <- list(order = seasonal) else
> >>>>("\'seasonal\' is of the wrong length")
> >>>> it should be rather:
> >>>> if (length(order) == 3) seasonal <- list(order = seasonal) else
> >>>> stop("\'seasonal\' is of the wrong length")
> >>>
> >>> I think you're right about this one.
>
> well, actually, the mishap is larger:
>
> Reading the help page for arima, 'seasonal' is documented as
>
> seasonal: A specification of the seasonal part of the ARIMA model, plus
>the period (which defaults to ‘frequency(x)’). This may be a
>list with components ‘order’ and ‘period’, or just a numeric
>vector of length 3 which specifies the seasonal ‘order’. In
>the latter case the default period is used.
>
> Note the
> or just a numeric vector of length 3 ... the seasonal 'order'
> part.
> If you look at the larger context of the
>
>else ("'seasonal...
>
> part, it becomes clear that -- in order to fulfill the above
> documented behavior, it's not length(order),
> but length(seasonal) which should be 3 which leads to the
> following change :
>
> @@ -124,10 +124,11 @@
> if(!is.numeric(seasonal$order) || length(seasonal$order) != 3L
> || any(seasonal$order < 0L))
> stop("'seasonal$order' must be a non-negative numeric
> vector of length 3")
> -} else if(is.numeric(order)) {
> -if(length(order) == 3L) seasonal <- list(order=seasonal)
> -else ("'seasonal' is of the wrong length")
> -} else stop("'seasonal' must be a list with component 'order'")
> +} else if(is.numeric(seasonal)) { # meant to be seasonal$order
> +if(length(seasonal) != 3L || any(seasonal < 0))
> +stop("if not a list, 'seasonal' must be a non-negative
> numeric vector of length 3")
> +seasonal <- list(order=seasonal)
> +} else stop("'seasonal' must be a list with component 'order' or
> length-3 vector")
>
>
> ... I still plan to commit this, but it may well be that this
> change will wake up arima() use that was buggy and never detected
> till now.
>
> Martin
>
>
>
[[alternative HTML version deleted]]
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
[Rd] Possible issue in stats/arima.R package
Hello all,
I am running R version 4.4.2 (2024-10-31 ucrt) on Windows 10 x64, and
noticed something that might be a minor bug (or at least inconsistent code)
in the stats/arima.R package.
I have found:
1. A missing stop() call at line 69:
if (length(order) == 3) seasonal <- list(order = seasonal) else
("\'seasonal\' is of the wrong length")
it should be rather:
if (length(order) == 3) seasonal <- list(order = seasonal) else
stop("\'seasonal\' is of the wrong length")
2. An unused 'mod' variable assignment at line 190:
mod <- makeARIMA(trarma[[1]], trarma[[2]], Delta, kappa, SSinit)
I am trying to confirm whether this is intended behavior or possibly an
overlooked detail. Could someone please clarify if the current logic is
correct?
Thank you.
Best regards,
Norbert Kuder
[[alternative HTML version deleted]]
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
