Re: [R-pkg-devel] Vignette build issue on Debian platform

2025-01-14 Thread Jisca Huisman

Hi Graeme,


There is only one function in my package that uses multiple cores:
bootSE(). However, this function is not called in any of my vignettes
because it is computationally expensive. The Rmd code has `eval = FALSE`
options for all of these cases. Therefore, I am at a loss as to why these
vignettes would be using more than 2 cores or what is causing this, or why
it only impacts the CRAN CMD check Debian system.


CRAN has changed its settings when compiling the vignette, and now 
*does* run everything in chunks with `eval = FALSE`.  The solution is to 
also add `purl=FALSE` to each chunk that should not be run.


If you dig through the mailing list archive you should be able to find 
further details on the why and how of this, or someone else might remember.


Best,

Jisca



On Tue, 14/01/2025 08:08, Graeme Hickey wrote:

Dear all,


I recently made some minor updates to the joineRML R package to squash some
CRAN CMD check NOTEs that have emerged over the past couple of years.  After
resubmitting to CRAN, a new NOTE appeared on the Debian platform only:



package joineRML_0.4.7.tar.gz does not pass the incoming checks

automatically, please see the following pre-tests (additional issue checks):


Windows: <

https://win-builder.r-project.org/incoming_pretest/joineRML_0.4.7_20250114_073418/Windows/00check.log

Status: OK
Debian: <

https://win-builder.r-project.org/incoming_pretest/joineRML_0.4.7_20250114_073418/Debian/00check.log

Status: 1 NOTE


The NOTE in particular is:



* checking re-building of vignette outputs ... [116s/30s] NOTE
Re-building vignettes had CPU time 3.9 times elapsed time


I emailed Uwe Ligges and got a reply:



We see
Flavor: r-devel-linux-x86_64-debian-gcc
Check: re-building of vignette outputs, Result: NOTE
Re-building vignettes had CPU time 3.9 times elapsed time



which suggests you are using more than 2 cores which is not permitted by

default.


There is only one function in my package that uses multiple cores:
bootSE(). However, this function is not called in any of my vignettes
because it is computationally expensive. The Rmd code has `eval = FALSE`
options for all of these cases. Therefore, I am at a loss as to why these
vignettes would be using more than 2 cores or what is causing this, or why
it only impacts the CRAN CMD check Debian system.


I noticed this has been a common reported issue (e.g.,
https://github.com/Rdatatable/data.table/issues/5658), but mainly for those
using data.table or openMP; I use neither. The same issue was showing for
some examples also, which I dealt with by wrapping in \dontrun{} — not
ideal, but seemed quickest way to avoid.


I have tried many things to fix this:

1. Modifying the vignettes to make faster
2. Adding various combinations of Sys.setenv("OMP_THREAD_LIMIT" = 1),
Sys.setenv("OMP_NUM_THREADS" = 1), options(Ncpus = 1), options(cores = 2)
to the Rmd vignettes
3. Checked the package builds using r-lib/actions (GitHub actions) and
r-hub Debian platform — I cannot reproduce this CRAN error


CRAN will not accept the update to my package until this NOTE is squashed.
If anyone is able to provide a recommendation on what I might do, I would
appreciate it.


Kind regards,


Graeme Hickey

[[alternative HTML version deleted]]

__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


[R-pkg-devel] Vignette build issue on Debian platform

2025-01-14 Thread Graeme Hickey
Dear all,


I recently made some minor updates to the joineRML R package to squash some
CRAN CMD check NOTEs that have emerged over the past couple of years.  After
resubmitting to CRAN, a new NOTE appeared on the Debian platform only:


> package joineRML_0.4.7.tar.gz does not pass the incoming checks
automatically, please see the following pre-tests (additional issue checks):

> Windows: <
https://win-builder.r-project.org/incoming_pretest/joineRML_0.4.7_20250114_073418/Windows/00check.log
>

> Status: OK

> Debian: <
https://win-builder.r-project.org/incoming_pretest/joineRML_0.4.7_20250114_073418/Debian/00check.log
>

> Status: 1 NOTE


The NOTE in particular is:


> * checking re-building of vignette outputs ... [116s/30s] NOTE

> Re-building vignettes had CPU time 3.9 times elapsed time


I emailed Uwe Ligges and got a reply:


> We see

> Flavor: r-devel-linux-x86_64-debian-gcc

> Check: re-building of vignette outputs, Result: NOTE

>Re-building vignettes had CPU time 3.9 times elapsed time


> which suggests you are using more than 2 cores which is not permitted by
default.


There is only one function in my package that uses multiple cores:
bootSE(). However, this function is not called in any of my vignettes
because it is computationally expensive. The Rmd code has `eval = FALSE`
options for all of these cases. Therefore, I am at a loss as to why these
vignettes would be using more than 2 cores or what is causing this, or why
it only impacts the CRAN CMD check Debian system.


I noticed this has been a common reported issue (e.g.,
https://github.com/Rdatatable/data.table/issues/5658), but mainly for those
using data.table or openMP; I use neither. The same issue was showing for
some examples also, which I dealt with by wrapping in \dontrun{} — not
ideal, but seemed quickest way to avoid.


I have tried many things to fix this:

   1. Modifying the vignettes to make faster
   2. Adding various combinations of Sys.setenv("OMP_THREAD_LIMIT" = 1),
   Sys.setenv("OMP_NUM_THREADS" = 1), options(Ncpus = 1), options(cores = 2)
   to the Rmd vignettes
   3. Checked the package builds using r-lib/actions (GitHub actions) and
   r-hub Debian platform — I cannot reproduce this CRAN error


CRAN will not accept the update to my package until this NOTE is squashed.
If anyone is able to provide a recommendation on what I might do, I would
appreciate it.


Kind regards,


Graeme Hickey

[[alternative HTML version deleted]]

__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


[R-pkg-devel] Replacement for SETLENGTH

2025-01-14 Thread Merlise Clyde, Ph.D.
I am trying to determine the best way to eliminate the use of SETLENGTH to 
truncate over allocated vectors in my package BAS to eliminate the NOTES about 
non-API calls in anticipation of R 4.5.0.

>From WRE:  "At times it can be useful to allocate a larger initial result 
>vector and resize it to a shorter length if that is sufficient. The functions 
>Rf_lengthgets and Rf_xlengthgets accomplish this; they are analogous to using 
>length(x) <- n in R. Typically these functions return a freshly allocated 
>object, but in some cases they may re-use the supplied object." 

it looks like using 

x = Rf_lengthgets(x, newsize);  
SET_VECTOR_ELT(result, 0, x); 

before returning works to resize without a performance hit that incurs with a 
copy.  (will this always re-use the supplied object if newsize < old size?)

There is no mention in section 5.9.2 about the need for re-protection of the 
object,  but it seems to be mentioned in some packages as well as a really old 
thread about SET_LENGTH that looks like a  non-API MACRO to lengthgets, 

indeed if I call gc() and then rerun my test I have had some non-reproducible 
aborts in R Studio on my M3 Mac (caught once in R -d lldb)  

Do I need to do something more like 

PROTECT_INDEX ipx0;.
PROTECT_WITH_INDEX(x0 = allocVector(REALSXP, old_size), &ipx0);

PROTECT_INDEX ipx1;.
PROTECT_WITH_INDEX(x1 = allocVector(REALSXP, old_size), &ipx1);

# fill in values in x0 and  x1up to new_size (random) < old_size
...
REPROTECT(x0 = Rf_lengthgets(x0, new_size), ipx0);
REPROTECT(x1 = Rf_lengthgets(x1, new_size), ipx1);

SET_VECTOR_ELT(result, 0, x0);
SET_VECTOR_ELT(result, 1, x1);
...
UNPROTECT(2);   # or is this 4? 
return(result);


There is also a mention in WRE of R_PreserveObject and R_ReleaseObject -  

looking for advice if this is needed, or which approach is better/more stable 
to replace SETLENGTH?   (I have many many instances that need to be updated, so 
trying to get some clarity here before updating and running code through 
valgrind or other sanitizers to catch any memory issues before submitting an 
update to CRAN.

best,
Merlise







__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


Re: [R-pkg-devel] Replacement for SETLENGTH

2025-01-14 Thread Iris Simmons
Hi Merlise!


Referring to here:

https://github.com/wch/r-source/blob/8ca367db0c94194f07ee7bcf4b883e9c5dc11e02/src/main/builtin.c#L832

It seems as though the object is only re-used if the new length is
equal to the old length.

If you use Rf_lengthgets, you will need to protect the return value.
The code you wrote that uses protect indexes looks correct, and the
reprotect is good because you no longer need the old object.

2 is the correct amount to unprotect. PROTECT and PROTECT_WITH_INDEX
(as far as I know) are the only functions that increase the size of
the protect stack, and so the only calls that need to be unprotected.
Typically, people define `int nprotect = 0;` at the start of their
functions, add `nprotect++;` after each PROTECT and PROTECT_WITH_INDEX
call, and add `UNPROTECT(nprotect);` immediately before each return or
function end. That makes it easier to keep track.

I typically use R_PreserveObject and R_ReleaseObject to protect
objects without a need to bind them somewhere in my package's
namespace. This would be that .onLoad() uses R_PreserveObject to
protect some objects, and .onUnload uses R_ReleaseObject to release
the protected objects. I probably would not use that for what you're
describing.


Regards,
Iris

On Tue, Jan 14, 2025 at 11:26 PM Merlise Clyde, Ph.D.  wrote:
>
> I am trying to determine the best way to eliminate the use of SETLENGTH to 
> truncate over allocated vectors in my package BAS to eliminate the NOTES 
> about non-API calls in anticipation of R 4.5.0.
>
> From WRE:  "At times it can be useful to allocate a larger initial result 
> vector and resize it to a shorter length if that is sufficient. The functions 
> Rf_lengthgets and Rf_xlengthgets accomplish this; they are analogous to using 
> length(x) <- n in R. Typically these functions return a freshly allocated 
> object, but in some cases they may re-use the supplied object."
>
> it looks like using
>
> x = Rf_lengthgets(x, newsize);
> SET_VECTOR_ELT(result, 0, x);
>
> before returning works to resize without a performance hit that incurs with a 
> copy.  (will this always re-use the supplied object if newsize < old size?)
>
> There is no mention in section 5.9.2 about the need for re-protection of the 
> object,  but it seems to be mentioned in some packages as well as a really 
> old thread about SET_LENGTH that looks like a  non-API MACRO to lengthgets,
>
> indeed if I call gc() and then rerun my test I have had some non-reproducible 
> aborts in R Studio on my M3 Mac (caught once in R -d lldb)
>
> Do I need to do something more like
>
> PROTECT_INDEX ipx0;.
> PROTECT_WITH_INDEX(x0 = allocVector(REALSXP, old_size), &ipx0);
>
> PROTECT_INDEX ipx1;.
> PROTECT_WITH_INDEX(x1 = allocVector(REALSXP, old_size), &ipx1);
>
> # fill in values in x0 and  x1up to new_size (random) < old_size
> ...
> REPROTECT(x0 = Rf_lengthgets(x0, new_size), ipx0);
> REPROTECT(x1 = Rf_lengthgets(x1, new_size), ipx1);
>
> SET_VECTOR_ELT(result, 0, x0);
> SET_VECTOR_ELT(result, 1, x1);
> ...
> UNPROTECT(2);   # or is this 4?
> return(result);
>
>
> There is also a mention in WRE of R_PreserveObject and R_ReleaseObject -
>
> looking for advice if this is needed, or which approach is better/more stable 
> to replace SETLENGTH?   (I have many many instances that need to be updated, 
> so trying to get some clarity here before updating and running code through 
> valgrind or other sanitizers to catch any memory issues before submitting an 
> update to CRAN.
>
> best,
> Merlise
>
>
>
>
>
>
>
> __
> R-package-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-package-devel

__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


Re: [R-pkg-devel] SystemRequirements & configure check for FFTW with single precision support

2025-01-14 Thread Simon Urbanek
Lukas,

I have not seen the communication so I'm not commenting on that specifically, I 
only looked at the GitHub link.

Although your configure could be improved (more below), it works well enough to 
detect fftw3f.

Unfortunately, SystemRequirements don't have a well-defined structure, but 
there are two commonly used notations:

a) library name and version so in your case that would be something like
libfftw3f (>=3.3.0)

b) deb/rpm package names
libfftw3-dev (deb), fftw3-devel (rpm)

fftw is a bit of a mess, because Debian ships the single-precision static 
library in libfftw3-dev, but also has libfftw3-single3 which is the dynamic 
version of the same, while RH does not distinguish. macOS recipe names are 
usually included only if they are non-obvious, but that would be the case here 
since installing fftw recipe does not work for you, so mentioning fftw-s is 
probably a good idea (there are community scripts that try to extract that 
information from the packages so that the dependencies can be installed).

As for your configure, my main objection would be that it ignores CPPFLAGS 
(they are not substituted at all even though they *are* used in the tests) and 
doesn't use pkg-config to get the correct flags. (Also the brew part should go 
- it's makes unwarranted assumptions [see below] and is entirely superfluous if 
you use pkg-config instead.) To illustrate what I mean, you get

$ pkg-config --cflags fftw3f
-I/opt/R/x86_64/include
$ pkg-config --libs fftw3f
-L/opt/R/x86_64/lib -lfftw3f -lm

while fCWTr on GitHub only uses

  Configuration for fCWTr 0.2.9000

cppflags:
cxxflags:
ldflags:   
libs: -lfftw3f

It actually works despite that, because R will inject the other flags for you, 
but that will only work is fftw is installed in the same location as the other 
libraries used by R (which is common, but not guaranteed). I wouldn't say it is 
dealbreaker, but it would recommend it for robustness.

FWIW with homebrew the correct flags are obtainable from pkg-config:

$ pkg-config --libs fftw3f
-L/opt/brew/Cellar/fftw/3.3.8_1/lib -lfftw3f
$ pkg-config --cflags fftw3f
-I/opt/brew/Cellar/fftw/3.3.8_1/include

Cheers,
Simon



> On Jan 11, 2025, at 10:37 PM, Lukas Schneiderbauer 
>  wrote:
> 
> Hi list,
> 
> I am working on getting a package  
> to
> CRAN. It depends on the FFTW library  that is built
> with single precision support. I am stuck in the submission process and I
> require your help.
> 
> Before I come to my questions, some key facts about the package:
> 
> The package has an autoconf configure script that uses (among other things,
> like OpenMP checks, etc..) AC_SEARCH_LIBS to check whether required
> functions of the library 'fftwf' exist, if yes, it adds the corresponding
> compiler/linker flags; if no, it errs with a descriptive error message.
> 
> The CRAN Windows as well as Linux build service included fftwf in their
> fftw build out of the box, and so building there was no problem, R CMD
> Check passes there. In the past, building for MacOS was more trouble, since
> its fftw package does not include a single-precision build. I reached out
> to Simon Urbanek, and he was so kind as to add an appropriate new recipe
> "fftw-s" that provided an fftw version with single precision support. As of
> now, R CMD check also passes cleanly on the MacOS build service, thanks to
> Simon Urbanek's efforts.
> 
> Now, I am stuck at submission for two reasons:
> 1. The SystemRequirements specification in the DESCRIPTION file is
> incorrect.
> 2. It is said that "the package needs a configure check for fftwf".
> 
> Add 1.
> Initially, I had no mention of the "single precision" version of fftw,
> because I thought it is included everywhere by default. It was stated that
> I need to add that information. I naturally complied.
> This is my current version:
> "SystemRequirements: fftw3 (including single precision support fftw3f),
> fftw3f_omp (optional), OpenMP (optional)"
> In the second subscription run, I was told to add "fftw-s" since I require
> the fftw-s package on MacOS. This does not make much sense to me since
> "fftw-s" is only the name of this package on Simon Urbanek's MacOS build
> service. The library file itself is still called fftwf, like it is on any
> other platform. If I added "fftw-s", I would also need to explain that this
> is only valid for MacOS which seems to make the SystemRequirements
> unnecessary verbose.
> * Can someone explain the reason behind this request to me?
> * How exactly should I add "fftw-s" to pass the submission process?
> 
> Add 2.
> I tried to explain now for the second time in the submission notes, that a
> check is already in place (see the AC_SEARCH_LIBS paragraph above). But my
> explanation gets ignored.
> * What am I doing wrong?
> * What additional configure checks do I need to add to the package?
> 
> Thanks a lot for your help!
> Sincerely, Lukas Schneiderba