Re: [Rd] stats::fft produces inconsistent results

2021-10-20 Thread Dipterix Wang
Wow, you guys are amazing!

>>> as part of its pipeline, ravetools::mvfftw computes the mean of the
>>> input vector **and then centers it to a mean of zero** (intentionally or
>>> accidentally?)
> 
>>> because variables are passed to compiled code by reference (someone
>>> can feel free to correct my terminology), this means that the original
>>> vector in R now has a mean of zero


I didn’t center the input vector in my code. The data was fed “as-is” into 
FFTW3. My guess is FFTW3 internally center the data. It could be that FFTW3 
library behave differently on different platforms, which could explain the 
reproducibility issue. 

>>> "Indeed, R vectors are passed "by reference" to C code, but the semantic 
>>> must be "by value", i.e. the C function must NOT change the contents of the 
>>> vector, except in very specific cases.”


CRAN has already had fftw and fftwtools, the issue is the data I’m targeting at 
are at GB-level, copying the vectors can be memory inefficient or even use up 
memories. The strategy of ravetools is to import signals from local files, fft, 
then directly write to disk. So only one reference will be used and modifying 
in-place is on purpose. In fact, and the fft functions I created are not 
intended to be used directly by users.

However, I should've been very cautious when using these functions. This is my 
fault. I’ll check the whole package to make sure only one reference is used or 
otherwise the vectors will be copied.

>>> This can be tested by the MAYBE_REFERENCED() macro in Rinternals.h.


Nice to learn! I’ll add it to my code.

>>> Properly using R vectors in C code is tricky. You have to understand.
>>> 1) When you are allowed or not to modify vectors
>>> 2) When to PROTECT()vectors
>>> 3) How the garbage collector works and when it can trigger (answer : 
>>> basically, when you call any internal R function)
>>> Chapter 5 of "Writing R Extensions" documentation is quite extensive:
>>>  
>>> https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Handling-R-objects-in-C
>>>  
>>> 

Indeed, I found myself often confused about when to PROTECT and when not. 

 ... but then ravetools  is not even a CRAN package, so why should you dare 
 to use it for anything serious ?

Haha, thanks : ) I guess I will probably be grouchy too if seeing so many 
people making the same mistakes again and again. It just happened to be me.
But it’s good to be rigorous. Sooner or later I'll have to face these problems. 
It’s better to make mistakes before having made many.

Thanks y’all!

Best,
- Dipterix Wang


> On Oct 20, 2021, at 5:32 AM, Martin Maechler  
> wrote:
> 
>> Martin Maechler 
>>on Wed, 20 Oct 2021 11:26:21 +0200 writes:
> 
> []
> 
>> Thank you, André , that's very good.
> 
>> Just to state the obvious conclusion:
> 
>> If Ben's suggestion is correct (and André has explained *how*
>> that could happen) this would mean  a
>> SEVERE BUG in package ravetools's  mvfftw() function.
> 
>> and it would have been (yet another) case of gaining speed by
>> killing correctness...
> 
>> ... but then ravetools  is not even a CRAN package, so why
>> should you dare to use it for anything serious ?
> 
>> ... yes, being grouchy ..
> 
> which I should rather not be.
> 
> Dipterix Wang *did* say initially that he is currently
> developing ravetools so it's very reasonabl this is not yet a
> CRAN package..
> 
> Best,
> Martin
> 


[[alternative HTML version deleted]]

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


Re: [Rd] Fwd: Using existing envars in Renviron on friendly Windows

2021-10-20 Thread Henrik Bengtsson
Two comments/suggestions:

1. What about recommending to always quote the value in Renviron
files, e.g. ABC="Hello world" and DEF="${APPDATA}/R-library"?  This
should a practice that works on all platforms.

2. What about having readRenviron() escapes strings it imports via
environment variables?  See example below.  Is there ever a use case
where someone wants/needs, or even rely on, the current behavior? (I
would even like to argue the current behavior is a design bug that
should be fixed.)  As an analogue from the shell world, Bash escapes
its input.

To illustrate the latter, with:

A=C:\\ABC
B=${A}
C="${A}"

or equivalently:

A="C:\ABC"
B=${A}
C="${A}"

we currently get:

$ Rscript -e "Sys.getenv(c('A', 'B', 'C'))"
A B C
"C:\\ABC"   "C:ABC" "C:\\ABC"

If base::readRenviron() would escape "input" environment variables, we
would get identical values for both 'B' and 'C', which I think is what
most people would expect.

To be clear, this is a problem that occur on all platforms, but it's
more likely to be revealed on MS Windows since paths uses backslashes,
but you could image a Linux user using something like
A="Hello\nworld\n" and would also be surprised about the above
behavior, when they end up with B="Hellonworldn".

/Henrik

On Wed, Oct 20, 2021 at 7:31 AM Michał Bojanowski  wrote:
>
> Hello Tomas,
>
> Yes, that's accurate although rather terse, which is perhaps the
> reason why I did not realize it applies to my case.
>
> How about adding something in the direction of:
>
> 1. Continuing the cited paragraph with:
> In particular, on Windows it may be necessary to quote references to
> existing environment variables, especially those containing file paths
> (which include backslashes). For example: `"${WINVAR}"`.
>
> 2. Add an example (not run):
>
> # On Windows do quote references to variables containing paths, e.g.:
> # If APPDATA=C:\Users\foobar\AppData\Roaming
> # to point to a library tree inside APPDATA in .Renviron use
> R_LIBS_USER="${APPDATA}"/R-library
>
> Incidentally the last example is on backslashes too.
>
> What do you think?
>
> On Mon, Oct 18, 2021 at 5:02 PM Tomas Kalibera  
> wrote:
> >
> >
> > On 10/15/21 6:44 PM, Michał Bojanowski wrote:
> > > Perhaps a small update to ?.Renviron would be in order to mention that...
> >
> > Would you have a more specific suggestion how to update the
> > documentation? Please note that it already says
> >
> > "‘value’ is then processed in a similar way to a Unix shell: in
> > particular the outermost level of (single or double) quotes is stripped,
> > and backslashes are removed except inside quotes."
> >
> > Thanks,
> > Tomas
> >
> > > On Fri, Oct 15, 2021 at 6:43 PM Michał Bojanowski  
> > > wrote:
> > >> Indeed quoting works! Kevin suggested the same, but he didnt reply to 
> > >> the list.
> > >> Thank you all!
> > >> Michal
> > >>
> > >> On Fri, Oct 15, 2021 at 6:40 PM Ivan Krylov  
> > >> wrote:
> > >>> Sorry for the noise! I wasn't supposed to send my previous message.
> > >>>
> > >>> On Fri, 15 Oct 2021 16:44:28 +0200
> > >>> Michał Bojanowski  wrote:
> > >>>
> >  AVAR=${APPDATA}/foo/bar
> > 
> >  Which is a documented way of referring to existing environment
> >  variables. Now, with that in R I'm getting:
> > 
> >  Sys.getenv("APPDATA")# That works OK
> >  [1] "C:\\Users\\mbojanowski\\AppData\\Roaming"
> > 
> >  so OK, but:
> > 
> >  Sys.getenv("AVAR")
> >  [1] "C:UsersmbojanowskiAppDataRoaming/foo/bar"
> > >>> Hmm, a function called by readRenviron does seem to remove backslashes,
> > >>> but not if they are encountered inside quotes:
> > >>>
> > >>> https://github.com/r-devel/r-svn/blob/3f8b75857fb1397f9f3ceab6c75554e1a5386adc/src/main/Renviron.c#L149
> > >>>
> > >>> Would AVAR="${APPDATA}"/foo/bar work?
> > >>>
> > >>> --
> > >>> Best regards,
> > >>> Ivan
> > > __
> > > R-devel@r-project.org mailing list
> > > https://stat.ethz.ch/mailman/listinfo/r-devel
>
> __
> R-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

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


[Rd] as.list.factor() should shift input names onto the resulting list

2021-10-20 Thread Davis Vaughan
Hi all,

The current implementation of as.list.factor() looks like:

as.list.factor
#> function (x, ...)
#> {
#> res <- vector("list", length(x))
#> for (i in seq_along(x)) res[[i]] <- x[i]
#> res
#> }
#> 
#> 

I believe this is incorrect, as names of `x` are not shifted onto `res`.
This results in the behavior shown below, which I am fairly certain is
incorrect (compared to Date methods and other classes). It also affects the
output of lapply().

I believe that as.list.factor() could be rewritten to be more like either
as.list.Date() or as.list.POSIXct(), which would fix the issue.

x <- factor(c("f1", "f2"))
names(x) <- c("a", "b")

y <- as.Date("2019-01-01") + 0:1
names(y) <- c("a", "b")

# Incorrect behavior:
# - Names are kept on inner elements
# - Names are not propagated onto resulting list
as.list(x)
#> [[1]]
#>  a
#> f1
#> Levels: f1 f2
#>
#> [[2]]
#>  b
#> f2
#> Levels: f1 f2

# Correct behavior:
# - Names are stripped from inner elements
# - Names are propagated onto resulting list
as.list(y)
#> $a
#> [1] "2019-01-01"
#>
#> $b
#> [1] "2019-01-02"

# The factor behavior breaks the lapply() invariant that names
# of `X` will be propagated onto the result
lapply(x, identity)
#> [[1]]
#>  a
#> f1
#> Levels: f1 f2
#>
#> [[2]]
#>  b
#> f2
#> Levels: f1 f2

# This works correctly
lapply(y, identity)
#> $a
#> [1] "2019-01-01"
#>
#> $b
#> [1] "2019-01-02"

Thanks,
Davis Vaughan

[[alternative HTML version deleted]]

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


[Rd] BUG?: R CMD check with --as-cran *disables* checks for unused imports otherwise performed

2021-10-20 Thread Henrik Bengtsson
ISSUE:

Using 'R CMD check' with --as-cran,
set_R_CHECK_PACKAGES_USED_IGNORE_UNUSED_IMPORTS_=TRUE, whereas the
default is FALSE, which you get if you don't add --as-cran.
I would expect --as-cran to check more things and more be conservative
than without.  So, is this behavior a mistake?  Could it be a thinko
around the negating "IGNORE", and the behavior is meant to be vice
verse?

Example:

$ R CMD check QDNAseq_1.29.4.tar.gz
...
* using R version 4.1.1 (2021-08-10)
* using platform: x86_64-pc-linux-gnu (64-bit)
...
* checking dependencies in R code ... NOTE
Namespace in Imports field not imported from: ‘future’
  All declared Imports should be used.

whereas, if I run with --as-cran, I don't get that NOTE;

$ R CMD check --as-cran QDNAseq_1.29.4.tar.gz
...
* checking dependencies in R code ... OK


TROUBLESHOOTING:

In src/library/tools/R/check.R [1], the following is set if --as-cran is used:

  Sys.setenv("_R_CHECK_PACKAGES_USED_IGNORE_UNUSED_IMPORTS_" = "TRUE")

whereas, if not set, the default is:

ignore_unused_imports <-
config_val_to_logical(Sys.getenv("_R_CHECK_PACKAGES_USED_IGNORE_UNUSED_IMPORTS_",
"FALSE"))

[1] 
https://github.com/wch/r-source/blob/b50e3f755674cbb697a4a7395b766647a5cfeea2/src/library/tools/R/check.R#L6335
[2] 
https://github.com/wch/r-source/blob/b50e3f755674cbb697a4a7395b766647a5cfeea2/src/library/tools/R/QC.R#L5954-L5956

/Henrik

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


[Rd] R Contribution Working Group

2021-10-20 Thread Heather Turner
Dear All,

The R Contribution Working Group was set up last year with the purpose of 
encouraging new contributors to R core, especially from currently 
under-represented groups. More detail here: 
https://forwards.github.io/rcontribution/working-group.

The group has been meeting approximately once a month with representatives from 
R Core, Forwards, R-Ladies, MiR and the general R contributor community (from 
aspiring to experienced contributors).  We currently alternate between a 
Americas-friendly time and a Europe/Middle East/Africa-friendly time. Anyone 
supporting the aims of the group is welcome to attend when they can.

The next meeting will be will be Tuesday, October 26, 2021, 20:00-21:00 UTC.

The agenda is here: https://hackmd.io/GzIGWM4ZTdmM3B3q6C24RA?edit - feel free 
to add items for us to discuss (time allowing!).

Join Zoom Meeting
https://us02web.zoom.us/j/88955759010?pwd=VW9IR2p1eVRicHc5czJSZ1VkUDB6QT09 (ID: 
88955759010, passcode: KOI9wWzh).

Best wishes,

Heather

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


Re: [Rd] Fwd: Using existing envars in Renviron on friendly Windows

2021-10-20 Thread Michał Bojanowski
Hello Tomas,

Yes, that's accurate although rather terse, which is perhaps the
reason why I did not realize it applies to my case.

How about adding something in the direction of:

1. Continuing the cited paragraph with:
In particular, on Windows it may be necessary to quote references to
existing environment variables, especially those containing file paths
(which include backslashes). For example: `"${WINVAR}"`.

2. Add an example (not run):

# On Windows do quote references to variables containing paths, e.g.:
# If APPDATA=C:\Users\foobar\AppData\Roaming
# to point to a library tree inside APPDATA in .Renviron use
R_LIBS_USER="${APPDATA}"/R-library

Incidentally the last example is on backslashes too.

What do you think?

On Mon, Oct 18, 2021 at 5:02 PM Tomas Kalibera  wrote:
>
>
> On 10/15/21 6:44 PM, Michał Bojanowski wrote:
> > Perhaps a small update to ?.Renviron would be in order to mention that...
>
> Would you have a more specific suggestion how to update the
> documentation? Please note that it already says
>
> "‘value’ is then processed in a similar way to a Unix shell: in
> particular the outermost level of (single or double) quotes is stripped,
> and backslashes are removed except inside quotes."
>
> Thanks,
> Tomas
>
> > On Fri, Oct 15, 2021 at 6:43 PM Michał Bojanowski  
> > wrote:
> >> Indeed quoting works! Kevin suggested the same, but he didnt reply to the 
> >> list.
> >> Thank you all!
> >> Michal
> >>
> >> On Fri, Oct 15, 2021 at 6:40 PM Ivan Krylov  wrote:
> >>> Sorry for the noise! I wasn't supposed to send my previous message.
> >>>
> >>> On Fri, 15 Oct 2021 16:44:28 +0200
> >>> Michał Bojanowski  wrote:
> >>>
>  AVAR=${APPDATA}/foo/bar
> 
>  Which is a documented way of referring to existing environment
>  variables. Now, with that in R I'm getting:
> 
>  Sys.getenv("APPDATA")# That works OK
>  [1] "C:\\Users\\mbojanowski\\AppData\\Roaming"
> 
>  so OK, but:
> 
>  Sys.getenv("AVAR")
>  [1] "C:UsersmbojanowskiAppDataRoaming/foo/bar"
> >>> Hmm, a function called by readRenviron does seem to remove backslashes,
> >>> but not if they are encountered inside quotes:
> >>>
> >>> https://github.com/r-devel/r-svn/blob/3f8b75857fb1397f9f3ceab6c75554e1a5386adc/src/main/Renviron.c#L149
> >>>
> >>> Would AVAR="${APPDATA}"/foo/bar work?
> >>>
> >>> --
> >>> Best regards,
> >>> Ivan
> > __
> > R-devel@r-project.org mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel

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


Re: [Rd] Environment setting _R_CHECK_DEPENDS_ONLY_='true'

2021-10-20 Thread Dirk Eddelbuettel


On 20 October 2021 at 09:31, Sebastian Meyer wrote:
| If you set the environment variable inside a running R process, it will 
| only affect that process and child processes, but not an independent R 
| process launched from a shell like you seem to be doing here:

Yes. That is somewhat common, if obscure, knowledge by those bitten before.

Maybe a line or two could be / should be added to the docs to that effect?

| How to set environment variables is system-specific. On a Unix-like 
| system, you could use the command
| 
| _R_CHECK_DEPENDS_ONLY_=true  R CMD check qra_0.2.4.tar.gz
| 
| to set the environment variable for this R process.
| See, e.g., https://en.wikipedia.org/wiki/Environment_variable.

R does have hooks for this, I had these for a few years now:

  ~/.R/check.Renviron
  ~/.R/check.Renviron-Rdevel

Again, might be worthwhile documenting it in the Inst+Admin manual (if it
isn' already, I don't recall right now).

Dirk

-- 
https://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org

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


Re: [Rd] stats::fft produces inconsistent results

2021-10-20 Thread Martin Maechler
> Martin Maechler 
> on Wed, 20 Oct 2021 11:26:21 +0200 writes:

[]

> Thank you, André , that's very good.

> Just to state the obvious conclusion:

> If Ben's suggestion is correct (and André has explained *how*
> that could happen) this would mean  a
> SEVERE BUG in package ravetools's  mvfftw() function.

> and it would have been (yet another) case of gaining speed by
> killing correctness...

> ... but then ravetools  is not even a CRAN package, so why
> should you dare to use it for anything serious ?

> ... yes, being grouchy ..

which I should rather not be.

Dipterix Wang *did* say initially that he is currently
developing ravetools so it's very reasonabl this is not yet a
CRAN package..

Best,
Martin

>> -Message d'origine-
>> De : R-devel  De la part de Ben Bolker
>> Envoyé : mercredi 20 octobre 2021 03:27
>> À : r-devel@r-project.org
>> Objet : Re: [Rd] stats::fft produces inconsistent results


>> This is a long shot, but here's a plausible scenario:

>> as part of its pipeline, ravetools::mvfftw computes the mean of the
>> input vector **and then centers it to a mean of zero** (intentionally or
>> accidentally?)

>> because variables are passed to compiled code by reference (someone
>> can feel free to correct my terminology), this means that the original
>> vector in R now has a mean of zero

>> the first element of fft() is mean(x)*length(x), so if mean(x) has
>> been forced to zero, that would explain your issue.

>> I don't know about the non-reproducibility part.

>> On 10/19/21 7:06 PM, Dipterix Wang wrote:
>>> Dear R-devel Team,
>>> 
>>> I'm developing a neuroscience signal pipeline package in R 
(https://github.com/dipterix/ravetools) and I noticed a weird issue that failed 
my unit test.
>>> 
>>> Basically I was trying to use `fftw3` library to implement fast 
multivariate fft function in C++. When I tried to compare my results with 
stats::fft, the test result showed the first element of **expected** (which was 
produced by stats::fft) was zero, which, I am pretty sure, is wrong, and I can 
confirm that my function produces correct results.
>>> 
>>> However, somehow I couldn’t reproduce this issue on my personal 
computer (osx, M1, R4.1.1), the error simply went away.
>>> 
>>> The catch is my function produced consistent and correct results but 
stats::fft was not. This does not mean `stats::fft` has bugs. Instead, I 
suspect there could be some weird interactions between my code and stats::fft 
at C/C++ level, but I couldn’t figure it out why.
>>> 
>>> +++ Details:
>>> 
>>> Here’s the code I used for the test:
>>> 
>>> 
https://github.com/dipterix/ravetools/blob/4dc35d64763304aff869d92dddad38a7f2b30637/tests/testthat/test-fftw.R#L33-L41
>>> 
>>> Test code
>>> set.seed(1)
>>> x <- rnorm(1000)
>>> dim(x) <- c(100,10)
>>> a <- ravetools:::mvfftw_r2c(x, 0)
>>> c <- apply(x, 2, stats::fft)[1:51,]
>>> expect_equal(a, c)
>>> 
>>> 
>>> Here are the tests that gave me the errors:
>>> 
>>> The test logs on win-builder
>>> https://win-builder.r-project.org/07586ios8AbL/00check.log
>>> 
>>> Test logs on GitHub
>>> 
https://github.com/dipterix/ravetools/runs/3944874310?check_suite_focus=true
>>> 
>>> 
>>> —— Failed tests ——
>>> -- Failure (test-fftw.R:41:3): mvfftw_r2c 
--
>>> `a` (`actual`) not equal to `c` (`expected`).
>>> 
>>> actual vs expected
>>> [,1][,2]  [,3]  
[,4]...
>>> - actual[1, ] 10.8887367+ 0.000i  -3.7808077+ 0.000i   
2.967354+ 0.00i   5.160186+ 0.00i ...
>>> + expected[1, ]0.000+ 0.000i  -3.7808077+ 0.000i   
2.967354+ 0.00i   5.160186+ 0.00i...
>>> 
>>> 
>>> 
>>> The first columns are different, `actual` is the results I produced via 
`ravetools:::mvfftw_r2c`, and `expected` was produced by `stats::fft`
>>> 
>>> 
>>> Any help or attention is very much appreciated.
>>> Thanks,
>>> - Zhengjia

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

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


Re: [Rd] stats::fft produces inconsistent results

2021-10-20 Thread Martin Maechler
> GILLIBERT, Andre 
> on Wed, 20 Oct 2021 08:10:00 + writes:

> Hello,
> That sounds like a good diagnosis!
> Indeed, R vectors are passed "by reference" to C code, but the semantic 
must be "by value", i.e. the C function must NOT change the contents of the 
vector, except in very specific cases.

> A good program that has to work on a vector, must first duplicate the 
vector, unless the only reference to the vector is the reference inside the C 
function.
> This can be tested by the MAYBE_REFERENCED() macro in Rinternals.h.

> A good example can be found in the fft() function in 
src/library/stats/src/fourier.c in R source code:
> switch (TYPEOF(z)) {
> case INTSXP:
> case LGLSXP:
> case REALSXP:
> z = coerceVector(z, CPLXSXP);
> break;
> case CPLXSXP:
> if (MAYBE_REFERENCED(z)) z = duplicate(z);
> break;
> default:
> error(_("non-numeric argument"));
> }
> PROTECT(z);

> This code coerces non-complex vectors to complex. Since this makes a 
copy, there is no need to duplicate.
> Complex vectors are duplicated, unless they are not referenced by 
anything but the fft() function.

> Now, the z vector can be modified "in place" without inconsistency.

> Properly using R vectors in C code is tricky. You have to understand.
> 1) When you are allowed or not to modify vectors
> 2) When to PROTECT()vectors
> 3) How the garbage collector works and when it can trigger (answer : 
basically, when you call any internal R function)

> Chapter 5 of "Writing R Extensions" documentation is quite extensive:
> 
https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Handling-R-objects-in-C

> -- 
> Sincerely
> André GILLIBERT

Thank you, André , that's very good.

Just to state the obvious conclusion:

If Ben's suggestion is correct (and André has explained *how*
that could happen) this would mean  a
SEVERE BUG in package ravetools's  mvfftw() function.

and it would have been (yet another) case of gaining speed by
killing correctness...

... but then ravetools  is not even a CRAN package, so why
should you dare to use it for anything serious ?

... yes, being grouchy ..

> -Message d'origine-
> De : R-devel  De la part de Ben Bolker
> Envoyé : mercredi 20 octobre 2021 03:27
> À : r-devel@r-project.org
> Objet : Re: [Rd] stats::fft produces inconsistent results


> This is a long shot, but here's a plausible scenario:

> as part of its pipeline, ravetools::mvfftw computes the mean of the
> input vector **and then centers it to a mean of zero** (intentionally or
> accidentally?)

> because variables are passed to compiled code by reference (someone
> can feel free to correct my terminology), this means that the original
> vector in R now has a mean of zero

> the first element of fft() is mean(x)*length(x), so if mean(x) has
> been forced to zero, that would explain your issue.

> I don't know about the non-reproducibility part.

> On 10/19/21 7:06 PM, Dipterix Wang wrote:
>> Dear R-devel Team,
>> 
>> I'm developing a neuroscience signal pipeline package in R 
(https://github.com/dipterix/ravetools) and I noticed a weird issue that failed 
my unit test.
>> 
>> Basically I was trying to use `fftw3` library to implement fast 
multivariate fft function in C++. When I tried to compare my results with 
stats::fft, the test result showed the first element of **expected** (which was 
produced by stats::fft) was zero, which, I am pretty sure, is wrong, and I can 
confirm that my function produces correct results.
>> 
>> However, somehow I couldn’t reproduce this issue on my personal computer 
(osx, M1, R4.1.1), the error simply went away.
>> 
>> The catch is my function produced consistent and correct results but 
stats::fft was not. This does not mean `stats::fft` has bugs. Instead, I 
suspect there could be some weird interactions between my code and stats::fft 
at C/C++ level, but I couldn’t figure it out why.
>> 
>> +++ Details:
>> 
>> Here’s the code I used for the test:
>> 
>> 
https://github.com/dipterix/ravetools/blob/4dc35d64763304aff869d92dddad38a7f2b30637/tests/testthat/test-fftw.R#L33-L41
>> 
>> Test code
>> set.seed(1)
>> x <- rnorm(1000)
>> dim(x) <- c(100,10)
>> a <- ravetools:::mvfftw_r2c(x, 0)
>> c <- apply(x, 2, stats::fft)[1:51,]
>> expect_equal(a, c)
>> 
>> 
>> Here are the tests that gave me the errors:
>> 
>> The test logs on win-builder
>> https://win-builder.r-project.org/07586ios8AbL/00check.log
>> 
>> Test logs on GitHub
>> 
https://github.com/dipterix/ravetools/runs/3944874310?check_suite_focus=true
>> 
>> 
>> —— Failed tests ——
>> -- Failure (test-fftw.R:41:3): mvfftw_r2c 
---

Re: [Rd] stats::fft produces inconsistent results

2021-10-20 Thread GILLIBERT, Andre
Hello,

That sounds like a good diagnosis!
Indeed, R vectors are passed "by reference" to C code, but the semantic must be 
"by value", i.e. the C function must NOT change the contents of the vector, 
except in very specific cases.

A good program that has to work on a vector, must first duplicate the vector, 
unless the only reference to the vector is the reference inside the C function.
This can be tested by the MAYBE_REFERENCED() macro in Rinternals.h.

A good example can be found in the fft() function in 
src/library/stats/src/fourier.c in R source code:
switch (TYPEOF(z)) {
case INTSXP:
case LGLSXP:
case REALSXP:
z = coerceVector(z, CPLXSXP);
break;
case CPLXSXP:
if (MAYBE_REFERENCED(z)) z = duplicate(z);
break;
default:
error(_("non-numeric argument"));
}
PROTECT(z);

This code coerces non-complex vectors to complex. Since this makes a copy, 
there is no need to duplicate.
Complex vectors are duplicated, unless they are not referenced by anything but 
the fft() function.

Now, the z vector can be modified "in place" without inconsistency.

Properly using R vectors in C code is tricky. You have to understand.
1) When you are allowed or not to modify vectors
2) When to PROTECT()vectors
3) How the garbage collector works and when it can trigger (answer : basically, 
when you call any internal R function)

Chapter 5 of "Writing R Extensions" documentation is quite extensive:
https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Handling-R-objects-in-C

-- 
Sincerely
André GILLIBERT

-Message d'origine-
De : R-devel  De la part de Ben Bolker
Envoyé : mercredi 20 octobre 2021 03:27
À : r-devel@r-project.org
Objet : Re: [Rd] stats::fft produces inconsistent results


   This is a long shot, but here's a plausible scenario:

   as part of its pipeline, ravetools::mvfftw computes the mean of the
input vector **and then centers it to a mean of zero** (intentionally or
accidentally?)

   because variables are passed to compiled code by reference (someone
can feel free to correct my terminology), this means that the original
vector in R now has a mean of zero

   the first element of fft() is mean(x)*length(x), so if mean(x) has
been forced to zero, that would explain your issue.

   I don't know about the non-reproducibility part.

On 10/19/21 7:06 PM, Dipterix Wang wrote:
> Dear R-devel Team,
>
> I'm developing a neuroscience signal pipeline package in R 
> (https://github.com/dipterix/ravetools) and I noticed a weird issue that 
> failed my unit test.
>
> Basically I was trying to use `fftw3` library to implement fast multivariate 
> fft function in C++. When I tried to compare my results with stats::fft, the 
> test result showed the first element of **expected** (which was produced by 
> stats::fft) was zero, which, I am pretty sure, is wrong, and I can confirm 
> that my function produces correct results.
>
> However, somehow I couldn’t reproduce this issue on my personal computer 
> (osx, M1, R4.1.1), the error simply went away.
>
> The catch is my function produced consistent and correct results but 
> stats::fft was not. This does not mean `stats::fft` has bugs. Instead, I 
> suspect there could be some weird interactions between my code and stats::fft 
> at C/C++ level, but I couldn’t figure it out why.
>
> +++ Details:
>
> Here’s the code I used for the test:
>
> https://github.com/dipterix/ravetools/blob/4dc35d64763304aff869d92dddad38a7f2b30637/tests/testthat/test-fftw.R#L33-L41
>
> Test code
> set.seed(1)
> x <- rnorm(1000)
> dim(x) <- c(100,10)
> a <- ravetools:::mvfftw_r2c(x, 0)
> c <- apply(x, 2, stats::fft)[1:51,]
> expect_equal(a, c)
> 
>
> Here are the tests that gave me the errors:
>
> The test logs on win-builder
> https://win-builder.r-project.org/07586ios8AbL/00check.log
>
> Test logs on GitHub
> https://github.com/dipterix/ravetools/runs/3944874310?check_suite_focus=true
>
>
> —— Failed tests ——
>   -- Failure (test-fftw.R:41:3): mvfftw_r2c 
> --
>   `a` (`actual`) not equal to `c` (`expected`).
>
>   actual vs expected
>   [,1][,2]
>   [,3]  [,4]...
>   - actual[1, ] 10.8887367+ 0.000i  -3.7808077+ 0.000i   
> 2.967354+ 0.00i   5.160186+ 0.00i ...
>   + expected[1, ]0.000+ 0.000i  -3.7808077+ 0.000i   
> 2.967354+ 0.00i   5.160186+ 0.00i...
>
> 
>
> The first columns are different, `actual` is the results I produced via 
> `ravetools:::mvfftw_r2c`, and `expected` was produced by `stats::fft`
>
>
> Any help or attention is very much appreciated.
> Thanks,
> - Zhengjia
> __
> R-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

--
Dr. Benjamin Bolker
Profess

Re: [Rd] Environment setting _R_CHECK_DEPENDS_ONLY_='true'

2021-10-20 Thread Sebastian Meyer

(this should have been posted to R-package-devel, not R-devel)

Am 20.10.21 um 00:17 schrieb John Maindonald via R-devel:

Setting
Sys,setenv('_R_CHECK_DEPENDS_ONLY_'=‘true’)
or Sys,setenv('_R_CHECK_DEPENDS_ONLY_’=TRUE)

(either appear to be acceptable) appears to have no effect when I do, e.g.


If you set the environment variable inside a running R process, it will 
only affect that process and child processes, but not an independent R 
process launched from a shell like you seem to be doing here:




$R CMD check qra_0.2.4.tar.gz
* using log directory ‘/Users/johnm1/pkgs/qra.Rcheck’
* using R version 4.1.1 (2021-08-10)
* using platform: x86_64-apple-darwin17.0 (64-bit)
* using session charset: UTF-8
. . .

(This should have failed.)


How to set environment variables is system-specific. On a Unix-like 
system, you could use the command


_R_CHECK_DEPENDS_ONLY_=true  R CMD check qra_0.2.4.tar.gz

to set the environment variable for this R process.
See, e.g., https://en.wikipedia.org/wiki/Environment_variable.

Best regards,

Sebastian Meyer




I’d have expected that the "On most systems . . .” mentioned in the Writing R 
extensions
manual (1.1.3.1 Suggested packages) would include my setup.

Any insight on what I am missing will be welcome.

John Maindonald email: john.maindon...@anu.edu.au




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



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