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

install_unit() that is equivalent to an existing unit - different behaviour on Windows #301

Closed
ateucher opened this issue Feb 10, 2022 · 22 comments · Fixed by #302
Closed

Comments

@ateucher
Copy link

I would like to be able to install a unit that is equivalent (identical) to an already installed unit. This works fine on macOS and Linux, but fails on Windows

Using the latest version of {units} - I've confirmed this with on macOS with the binary CRAN version and the latest github version (built with the latest {udunits2} from homebrew), but it fails on Windows.

macOS

library(units)
#> udunits database from /usr/local/share/udunits/udunits2.xml

install_unit("foobar", "1 kg", "A foobar")
x <- as_units(5, "foobar")
x
#> 5 [foobar]
units(x) <- "g"
x
#> 5000 [g]

Windows

units::install_unit("foobar", "1 kg", "A foobar")
#> Error in ud_map_symbols(symbol, ut_unit) : Unit already maps to "kg"
#> Backtrace:
#> x
#> 1. \-units::install_unit("foobar", "1 kg", "A foobar")
#> 2. \-units:::ud_map_symbols(symbol, ut_unit)

I notice that {units} on Windows is built against an old version of udunits2 (v2.2.20), whereas it is built against the current (v2.2.28) version on macOS, so I'm guessing the difference might lie there?

@Enchufa2
Copy link
Member

Can you provide please the sessionInfo for both systems?

@ateucher
Copy link
Author

Here it is for my Mac, my colleague @boshek will post his for Windows...

R version 4.1.2 (2021-11-01)
Platform: x86_64-apple-darwin17.0 (64-bit)
Running under: macOS Big Sur 11.6.3

Matrix products: default
LAPACK: /Library/Frameworks/R.framework/Versions/4.1/Resources/lib/libRlapack.dylib

locale:
[1] en_CA.UTF-8/en_CA.UTF-8/en_CA.UTF-8/C/en_CA.UTF-8/en_CA.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] units_0.8-0

loaded via a namespace (and not attached):
[1] compiler_4.1.2 cli_3.1.1      tools_4.1.2    Rcpp_1.0.8     rlang_1.0.1

@boshek
Copy link

boshek commented Feb 10, 2022

For the windows system:

R> sessioninfo::session_info()
- Session info -------------------------------------------------------------------------------------------------
 setting  value
 version  R version 4.1.2 (2021-11-01)
 os       Windows 10 x64 (build 19043)
 system   x86_64, mingw32
 ui       RStudio
 language (EN)
 collate  English_Canada.1252
 ctype    English_Canada.1252
 tz       America/Los_Angeles
 date     2022-02-10
 rstudio  1.4.1717 Juliet Rose (desktop)
 pandoc   2.11.4 @ C:\\PROGRA~1\\RStudio\\bin\\pandoc\\pandoc.exe

- Packages -----------------------------------------------------------------------------------------------------
 package     * version date (UTC) lib source
 cli           3.1.1   2022-01-20 [1] CRAN (R 4.1.2)
 crayon        1.4.2   2021-10-29 [1] CRAN (R 4.1.1)
 ellipsis      0.3.2   2021-04-29 [1] CRAN (R 4.0.5)
 fansi         1.0.2   2022-01-14 [1] CRAN (R 4.1.2)
 glue          1.6.1   2022-01-22 [1] CRAN (R 4.1.2)
 httr          1.4.2   2020-07-20 [1] CRAN (R 4.0.2)
 lifecycle     1.0.1   2021-09-24 [1] CRAN (R 4.1.0)
 magrittr      2.0.2   2022-01-26 [1] CRAN (R 4.1.2)
 pillar        1.7.0   2022-02-01 [1] CRAN (R 4.1.2)
 pkgconfig     2.0.3   2019-09-22 [1] CRAN (R 4.0.0)
 R6            2.5.1   2021-08-19 [1] CRAN (R 4.1.0)
 Rcpp          1.0.8   2022-01-13 [1] CRAN (R 4.1.2)
 rlang         1.0.0   2022-01-26 [1] CRAN (R 4.1.2)
 rstudioapi    0.13    2020-11-12 [1] CRAN (R 4.0.3)
 rvest         1.0.2   2021-10-16 [1] CRAN (R 4.1.1)
 sessioninfo   1.2.2   2021-12-06 [1] CRAN (R 4.1.2)
 tibble        3.1.6   2021-11-07 [1] CRAN (R 4.1.2)
 units       * 0.8-0   2022-02-10 [1] Github (r-quantities/units@ff6b8ae)
 utf8          1.2.2   2021-07-24 [1] CRAN (R 4.1.0)
 vctrs         0.3.8   2021-04-29 [1] CRAN (R 4.0.5)
 xml2          1.3.3   2021-11-30 [1] CRAN (R 4.1.2)

 [1] C:/Users/salbers/R/win-library/4.0
 [2] C:/Users/salbers/R/win-library/4.1
 [3] C:/Program Files/R/R-4.1.2/library

----------------------------------------------------------------------------------------------------------------

@Enchufa2
Copy link
Member

Install the latest version from CRAN, please.

@boshek
Copy link

boshek commented Feb 10, 2022

Same result

units::install_unit("foobar", "1 kg", "A foobar")
#> Error in ud_map_symbols(symbol, ut_unit): Unit already maps to "kg"
Session info
sessioninfo::session_info()
#> - Session info ---------------------------------------------------------------
#>  setting  value
#>  version  R version 4.1.2 (2021-11-01)
#>  os       Windows 10 x64 (build 19043)
#>  system   x86_64, mingw32
#>  ui       RTerm
#>  language (EN)
#>  collate  English_Canada.1252
#>  ctype    English_Canada.1252
#>  tz       America/Los_Angeles
#>  date     2022-02-10
#>  pandoc   2.11.4 @ C:/Program Files/RStudio/bin/pandoc/ (via rmarkdown)
#> 
#> - Packages -------------------------------------------------------------------
#>  package     * version date (UTC) lib source
#>  backports     1.4.1   2021-12-13 [1] CRAN (R 4.1.2)
#>  cli           3.1.1   2022-01-20 [1] CRAN (R 4.1.2)
#>  crayon        1.4.2   2021-10-29 [1] CRAN (R 4.1.1)
#>  digest        0.6.29  2021-12-01 [1] CRAN (R 4.1.2)
#>  ellipsis      0.3.2   2021-04-29 [1] CRAN (R 4.0.5)
#>  evaluate      0.14    2019-05-28 [1] CRAN (R 4.0.0)
#>  fansi         1.0.2   2022-01-14 [1] CRAN (R 4.1.2)
#>  fastmap       1.1.0   2021-01-25 [1] CRAN (R 4.0.3)
#>  fs            1.5.2   2021-12-08 [1] CRAN (R 4.1.2)
#>  glue          1.6.1   2022-01-22 [1] CRAN (R 4.1.2)
#>  highr         0.9     2021-04-16 [1] CRAN (R 4.0.4)
#>  htmltools     0.5.2   2021-08-25 [1] CRAN (R 4.1.1)
#>  knitr         1.37    2021-12-16 [1] CRAN (R 4.1.2)
#>  lifecycle     1.0.1   2021-09-24 [1] CRAN (R 4.1.0)
#>  magrittr      2.0.2   2022-01-26 [1] CRAN (R 4.1.2)
#>  pillar        1.7.0   2022-02-01 [1] CRAN (R 4.1.2)
#>  pkgconfig     2.0.3   2019-09-22 [1] CRAN (R 4.0.0)
#>  purrr         0.3.4   2020-04-17 [1] CRAN (R 4.0.0)
#>  R.cache       0.15.0  2021-04-30 [1] CRAN (R 4.0.5)
#>  R.methodsS3   1.8.1   2020-08-26 [1] CRAN (R 4.0.2)
#>  R.oo          1.24.0  2020-08-26 [1] CRAN (R 4.0.2)
#>  R.utils       2.11.0  2021-09-26 [1] CRAN (R 4.1.0)
#>  Rcpp          1.0.8   2022-01-13 [1] CRAN (R 4.1.2)
#>  reprex        2.0.1   2021-08-05 [1] CRAN (R 4.1.0)
#>  rlang         1.0.0   2022-01-26 [1] CRAN (R 4.1.2)
#>  rmarkdown     2.11    2021-09-14 [1] CRAN (R 4.1.0)
#>  rstudioapi    0.13    2020-11-12 [1] CRAN (R 4.0.3)
#>  sessioninfo   1.2.2   2021-12-06 [1] CRAN (R 4.1.2)
#>  stringi       1.7.6   2021-11-29 [1] CRAN (R 4.1.2)
#>  stringr       1.4.0   2019-02-10 [1] CRAN (R 4.0.2)
#>  styler        1.6.2   2021-09-23 [1] CRAN (R 4.1.1)
#>  tibble        3.1.6   2021-11-07 [1] CRAN (R 4.1.2)
#>  units         0.8-0   2022-02-05 [1] CRAN (R 4.1.2)
#>  utf8          1.2.2   2021-07-24 [1] CRAN (R 4.1.0)
#>  vctrs         0.3.8   2021-04-29 [1] CRAN (R 4.0.5)
#>  withr         2.4.3   2021-11-30 [1] CRAN (R 4.1.0)
#>  xfun          0.29    2021-12-14 [1] CRAN (R 4.1.2)
#>  yaml          2.2.2   2022-01-25 [1] CRAN (R 4.1.2)
#> 
#>  [1] C:/Users/salbers/R/win-library/4.0
#>  [2] C:/Users/salbers/R/win-library/4.1
#>  [3] C:/Program Files/R/R-4.1.2/library
#> 
#> ------------------------------------------------------------------------------

@ateucher
Copy link
Author

I've forked the repo and created a test for this; It fails on Windows (as reported here), but passes on all other platforms: https://github.com/ateucher/units/runs/5149759655?check_suite_focus=true

@Enchufa2
Copy link
Member

Thanks, @ateucher, that's very useful. The only difference I see is the version of the library. udunits v2.2.20 dates October 2015. The only thing we can do is to ask CRAN to update it.

I was trying to see what changed in the library, but it's not obvious.

@Enchufa2
Copy link
Member

Nope, I installed v2.2.20 on Linux and it works just fine. The library does some things differently on Windows, so this is an upstream bug worth reporting.

@ateucher
Copy link
Author

ateucher commented Feb 11, 2022

That is interesting... though I feel like there's a chance that it did get fixed upstream in the updates between v2.2.20 and v2.2.28, so it may be worth asking if we can get the Windows udunits2 library updated regardless? I can ping @jeroen to see if we can update this, which as far as I can tell is what is used to build the package on Windows (https://github.com/r-quantities/units/blob/main/src/Makevars.win)? Or does CRAN use a different source of udunits2 to build binaries?

@Enchufa2
Copy link
Member

That is interesting... though I feel like there's a chance that it did get fixed upstream in the updates between v2.2.20 and v2.2.28

I don't think so. There's nothing even related in the changelog, and no significant changes in the functions involved. However, they use different code paths to search for units in Windows and everywhere else (see here). So I believe there's a bug in the Windows code path.

so it may be worth asking if we can get the Windows udunits2 library updated regardless?

2015 was... a long time ago. :) So yes, we could all benefit from an update regardless of this issue.

I can ping @jeroen to see if we can update this, which as far as I can tell is what is used to build the package on Windows? Or does CRAN use a different source of udunits2 to build binaries?

AFAIK, CRAN uses a separate toolchain for their checks, but Jeroen maintains rtools4? Not sure. And MSYS2 seems to have a newer version. So... I'm a bit lost here.

@jeroen
Copy link
Contributor

jeroen commented Feb 11, 2022

I will update the builds in rwinlib/udunits. There is also a newer version in the https://github.com/rwinlib/gdal3 repo, but that is maybe a bit overkill.

@ateucher
Copy link
Author

ateucher commented Feb 11, 2022

Thanks so much @jeroen! Are you (or @edzer?) able to shed some light on whether CRAN builds its Windows binaries with the udunits that is in MSYS2, or with that in rwinlib/udunits? I feel like the latter given the Makevars.win which calls this script, pulling from rwinlib/udunits? But CRAN's build processes are a bit opaque to me so I feel like there's a good chance I'm wrong...

@jeroen
Copy link
Contributor

jeroen commented Feb 11, 2022

For R 4.0 and 4.1 we download the version from rwinlib. For R 4.2, CRAN uses their own local copy.

@Enchufa2
Copy link
Member

@ateucher As I suspected, v2.2.28 doesn't solve the issue. Feel free to escalate this upstream, but in my experience, unfortunately they are not very responsive.

@Enchufa2 Enchufa2 reopened this Feb 11, 2022
@jeroen
Copy link
Contributor

jeroen commented Feb 11, 2022

Maybe it has to do with the version of the 'share' files. Let me try something.

@jeroen
Copy link
Contributor

jeroen commented Feb 11, 2022

No that wasn't the issue, never mind :)

@ateucher
Copy link
Author

Ah, that is unfortunate! Thanks so much for your help @jeroen and @Enchufa2

@Enchufa2
Copy link
Member

Reported upstream. It would be nice if @boshek could please compile the small example I put there on Windows to double check it fails.

@ateucher
Copy link
Author

So I gave this a shot on a Windows machine... compiling C/C++ is still pretty new to me, but this is what I did:

  1. Set up a msys2 system using Rtools4.0
  2. Installed udunits2 with pacman (this is the same library used the build the units package):
pacman -Sy
pacman -S mingw-w64-x86_64-udunits
  1. copied your example (slightly modified for the header location) to a file called test-units.c:
#include <stdio.h>
#include <udunits2.h>

int main() {
    ut_set_error_message_handler((ut_error_message_handler) ut_ignore);
    ut_system *sys = ut_read_xml(NULL);
    ut_encoding enc = UT_UTF8;
    ut_set_error_message_handler((ut_error_message_handler) vprintf);
    
    ut_unit *m = ut_parse(sys, "1 m", enc);
    ut_map_symbol_to_unit("my_metre", enc, m);
    // fails on Windows, works fine on Unix
    ut_map_unit_to_symbol(m, "my_metre", enc);
    // Error: Unit already maps to "m"
    
    ut_free(m);
    ut_free_system(sys);
    return 0;
}
  1. After a bit of experimenting and getting lots of errors locating the correct libraries etc. ran this:
gcc test-units.c -l udunits2 -l expat -IC:/rtools40/mingw64/include/

... and lo and behold, it ran with no error (returned exit code 0). So I don't know what's up now @Enchufa2 - any thoughts?

@ateucher
Copy link
Author

PS - For completeness I did this on my Mac and it was also fine...

@Enchufa2
Copy link
Member

Enchufa2 commented Mar 7, 2022

Sorry, I missed this. I'm a bit lost, because we have now a failure on Debian too, so I'm not sure what causes the issue anymore... :( If I'm not able to reproduce this, then I cannot debug it.

@Enchufa2
Copy link
Member

So... not only we didn't manage to reproduce this consistently, but now this now runs on Windows just fine too. :(

Closing here. But please feel free to reopen if you find the issue again.

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

Successfully merging a pull request may close this issue.

4 participants