pik-piam / gdx

R package | readGDX package for R
Other
1 stars 4 forks source link

have igdx() find GAMS on its own #6

Closed 0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q closed 9 months ago

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 9 months ago

We had trouble with tests being skipped because the GDX libraries were not loaded correctly (see this Mattermost discussion).

This code https://github.com/pik-piam/gdx/blob/ab8fa071c4f0a358605a2a191a7426e725dd3c62/R/onLoad.R#L8-L17 will fail silently (more accurately: report NULL) when there are no elements containing "gams" on the path (which was the case here).

At least since version 1.0.2 the gdxrrw uses improved mechanisms to find GAMS on its own, see here.

So this change uses that instead.

Output of the fail state (with unloaded gams module on the cluster) looks like

gamsSysDir arg is empty: no GDX API there
Environment variable R_GAMS_SYSDIR not set: no GDX API there
Error loading the GDX API via the default shared library search mechanism
Could not load shared library libgdxdclib64.so: libgdxdclib64.so: cannot open shared object file: No such file or directory
LD_LIBRARY_PATH = /p/system/packages/R/4.1.2/gcc-mkl/lib64/R/lib:/p/system/packages/intel/oneapi/2021.1.1/mkl/2021.1.1/lib/intel6...
The GDX library has not been loaded
0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 9 months ago

Crap. This fails on the cluster in general. (While I cannot make it fail for me locally, so it seemed quite robust.)

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 9 months ago

Options would be:

  1. Change to igdx(Sys.getenv("GAMSROOT")), which is defined on the cluster.
  2. setenv R_GAMS_SYSDIR /p/system/packages/gams/… in the gams module.
  3. append-path LD_LIBRARY_PATH /p/system/packages/gams/… in the gams module.
  4. Manually parse the PATH, which igdx() does on its own on Windows, but not on Linux.
tscheypidi commented 9 months ago

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q would it be an option to combine old and new methodology? Shouldn't that cover all known setups?

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 9 months ago

Sure, option 4.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 9 months ago

Will do that.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 9 months ago

I went with options 4 and 1. We could do options 2 and 3 on the cluster as well.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 9 months ago

Fixed an undocumented "feature" of R CMD check, which fails if packageStartupMessage() passes anything containing the string "Error".

pascal-sauer commented 8 months ago

I noticed recently R 4.4 builds on windows on r-universe fail with the fail state output, see e.g. here. Any ideas how to fix that @0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q ?

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 8 months ago

R version 4.4.0 alpha (2024-03-26 r86209 ucrt)

Interesting choice …

R 4.3 would complain if packageStartupMessage() contained the string "Error", see here. So either try to find the part of the message that is offending R 4.4, or call igdx() only when it is actually needed (inside readGDX() and writeGDX()), not when the package is loaded.