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

Unit test failure when running on i386 #384

Closed
tillea opened this issue Nov 28, 2022 · 12 comments
Closed

Unit test failure when running on i386 #384

tillea opened this issue Nov 28, 2022 · 12 comments
Milestone

Comments

@tillea
Copy link

tillea commented Nov 28, 2022

Description

I've updated the xts Debian package to the latest version (0.12.2). Debian builds and tests these packages automatically on different architectures. With the upload of version 0.12.2 the package fails on i386 as it is reported in Debian bug tracking system. A full build log is linked there. The errors are starting with

...
Loading required package: zoo

Attaching package: 'zoo'

The following objects are masked from 'package:base':

    as.Date, as.Date.numeric

Error in make.index.unique.xts(x, eps = eps) :
  (converted from warning) index value is unique but will be replaced; it is less than the cumulative epsilon for the preceding duplicate index values
In addition: Warning messages:
1: In make.index.unique.xts(x, eps = 1e-06) :
  index value is unique but will be replaced; it is less than the cumulative epsilon for the preceding duplicate index values
2: In make.index.unique.xts(x, eps = eps) :
  index value is unique but will be replaced; it is less than the cumulative epsilon for the preceding duplicate index values
Error in try.xts(sample.data.frame[, 1]) :
  Error in as.xts.double(x, ..., .RECLASS = TRUE) :   order.by must be either 'names()' or otherwise specified
Error in diff.xts(x, 1L, -1L) :
  'diff.xts' defined only for positive lag and differences arguments
Error in diff.xts(x, 1L, "a") : 'differences' must be integer
In addition: Warning message:
In diff.xts(x, 1L, "a") : NAs introduced by coercion
Error in diff.xts(x, -1L, 1L) :
  'diff.xts' defined only for positive lag and differences arguments
Error in diff.xts(x, "a", 1L) : 'lag' must be integer
In addition: Warning message:
In diff.xts(x, "a", 1L) : NAs introduced by coercion
Error in endpoints(x, on = "years", k = 0) : 'k' must be > 0
Error in endpoints(x, on = "years", k = -1) : 'k' must be > 0
Error in endpoints(x, on = "quarters", k = 0) : 'k' must be > 0

Expected behavior

I would expect that the tests would pass also on i386 as it was the case for version 0.12.1.

Minimal, reproducible example

Just run the test on some i386 machine

Kind regards, Andreas.

@joshuaulrich
Copy link
Owner

Thanks for the report Andreas! I'll take care of it. The rest of this information is for future me. I don't expect you to act on it.

All these "errors" are in the log because they're thrown as part of tests to check that errors are thrown when they should be. This version of the 'xts' package uses the 'RUnit' package for unit tests. 'RUnit' doesn't muffle errors when they're expected, so the log is cluttered with "Error in ..." messages. You have to look at the Unit Test Summary to identify any failing tests.

The relevant failing test is test.split_returns_named_list. This test was added in 0.12.2, so there is no regression from 0.12.1.

------------------- UNIT TEST SUMMARY ---------------------

RUNIT TEST PROTOCOL -- Sat Nov 12 17:18:54 2022 
*********************************************** 
Number of test functions: 431 
Number of errors: 0 
Number of failures: 1 

 
1 Test Suite : 
xts unit testing - 431 test functions, 0 errors, 1 failure
FAILURE in test.split_returns_named_list: Error in checkIdentical(nm_target, nm_ms, msg) : FALSE 
 microsecond data split by milliseconds
Error: 

unit testing failed (#test failures: 1, #R errors: 0)

My hypothesis is that the test fails on i386 because of precision issues. Here's a minimal example:

nm_minutes <- c("1970-01-01 00:00:00", "1970-01-01 00:01:00")
t1 <- as.POSIXct(nm_minutes[1], tz = "UTC")
us <- seq(1e-4, 2e-1, 1e-4)
x_us <- xts::xts(seq_along(us), t1 + us)
nm_ms <- names(split(x_us, "milliseconds"))
nm_target <- format(t1 + seq(0, 0.2, 0.001), "%Y-%m-%d %H:%M:%OS3")
RUnit::checkIdentical(nm_target, nm_ms, msg)

@joshuaulrich joshuaulrich changed the title Regression of test running on i386 Unit test failure when running on i386 Nov 28, 2022
@tillea
Copy link
Author

tillea commented Jan 12, 2023

Hi,
is there any progress in this? Regarding Debian release status its a bit urgent since we will freeze soon.
Kind regards, Andreas.

@joshuaulrich
Copy link
Owner

joshuaulrich commented Jan 12, 2023

Yikes, I'm sorry. I thought I took care of this...

@joshuaulrich
Copy link
Owner

I was able to replicate this on the i386/debian:latest docker container. Now this specific test only runs on 64-bit systems.

I'll start testing reverse-dependencies today, and should have an updated xts on CRAN by early next week.

Thanks again for the report!

@tillea
Copy link
Author

tillea commented Jan 12, 2023

Thanks for doing so.

@tillea
Copy link
Author

tillea commented Jan 19, 2023

Any success to track down the issue? I do not want to sound impatiently but the release process is creating some presure here at my side and some reverse dependencies of xts are affected.

@AdrianBunk
Copy link

@tillea I've got it passing by compiling R itself with -ffloat-store to mitigate the excess precision of the x87 FPU. I'll reassign the Debian bug with a patch in a while.

@joshuaulrich joshuaulrich added this to the 0.13.0 milestone Jan 19, 2023
@tillea
Copy link
Author

tillea commented Jan 19, 2023 via email

@joshuaulrich
Copy link
Owner

This is fixed in the development version on GitHub. I'm working on getting it released to CRAN as soon as possible. I ran into some reverse-dependency issues with other changes and I'm trying to fix/revert them.

@AdrianBunk
Copy link

@tillea Confirmed by my testing is that the difference between failing and passing autopkgtest of r-cran-xts is whether r-base-core is from Debian unstable or from the same sources recompiled with -ffloat-store.

-ffloat-store would cost some performance for FPU code in R on i386, but given that it's 2023 this should not matter much since i386 is mainline vintage hardware (multiarch usecases shouldn't apply here).

I would guess this change might also help with similar issues in other packages, but that is just a guess.

@tillea
Copy link
Author

tillea commented Jan 19, 2023 via email

@tillea
Copy link
Author

tillea commented Jan 19, 2023 via email

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