[Rd] all.equal applied to function closures

[Martin Maechler]

>>>>> Martin Maechler 
>>>>>     on Tue, 1 Dec 2020 10:31:36 +0100 writes:

>>>>> Bill Dunlap 
>>>>>     on Mon, 30 Nov 2020 13:41:54 -0800 writes:

    >> To make the comparison more complete,
    >> all.equal.environment could compare the parents of the
    >> target and current environments.  That would have to be
    >> recursive but could stop at the first 'top level
    >> environment' (the global, empty, or a package-related
    >> environment generally) and use identical there.  E.g.,

    >> > f1 <- function(x) (function(){ expx <- exp(x) ; function(y) y + expx})()
    >> > all.equal(f1(2), f1(3))
    >> [1] "Environments: Component “expx”: Mean relative difference: 1.718282"
    >> 
    >> [2] "Environments: <parent.env> Component “x”: Mean relative difference:
    >> 0.5"

    >> This is from the following, where I avoided putting the existing
    >> non-recursive all.equal.environment into the body of this one.

    [[.........]]

    > Thank you, Duncan and Bill (and Kevin for bringing up the
    > topic).

    > I agree  all.equal() should work better with functions,

    > and I think probably it would make sense to define  all.equal.function()
    > rather than put this into all.equal.default()

    > However, it's not quite clear if it is always desirable to check the
    > environments as well notably as that *is* done recursively.

A small  "work in progress" update (because I've been advancing slowly only):

I'm currently testing 'make check-all' after

1) adding all.equal.function() method,
2) not changing  all.equal.environment() yet
3) but adapting all.equal.default() to give a deprecating warning
   when called with a function

all.equal.function <- function(target, current, check.environments = TRUE, ...)
{
    msg <- all.equal.language(target, current, ...)
    if(check.environments) {
        ## pre-check identical(), for speed *and* against infinite recursion:
        ee <- identical(environment(target), environment(current), ignore.environment=FALSE)
        if(!ee)
            ee <- all.equal.environment(environment(target), environment(current), ...)
        if(isTRUE(msg))
            ee
        else
            c(msg, if(!isTRUE(ee)) ee)
    } else
        msg
}

It's amazing that this breaks our own checks in several places,
which I'm chasing slowly (being busy with teaching related
duties, etc).
I did have the correct gut feeling to have 'check.environments'
being an optional argument, because indeed there are cases (in
our own regression tests) where we now needed to change the
checks to use   check.environments=FALSE .

My plan is to finish the   all.equal.function()  functionality /
modify "base R" coded (or, for now, rather the testing code)
where needed, and then *commit* that to R-devel.

After that we should come back to improve or even re-write
all.equal.environment()  when needed.

Martin

    > Bill, I'm sure you've noticed that we did write  all.equal.environment()
    > to work recursively... Actually, I had worked quite a bit at
    > that, too long ago to remember details, but the relevant svn log
    > entry is
    > ------------------------------------------------------------------------
    > r66640 | maechler | 2014-09-18 22:10:20 +0200 (Thu, 18 Sep 2014) | 1 line

    > more sophisticated all.equal.environment(): no longer "simple" infinite recursions
    > ------------------------------------------------------------------------

    > Are you sure that code with the internal recursive do1()
    > function should/could not be amended where needed?

    > Martin

    >> On Mon, Nov 30, 2020 at 10:42 AM Duncan Murdoch <murdoch.duncan using gmail.com>
    >> wrote:
    >> 
    >> > On 30/11/2020 1:05 p.m., Kevin Van Horn via R-devel wrote:
    >> > > Consider the following code:
    >> > >
    >> > >      f <- function(x)function(y){x+y}
    >> > >      all.equal(f(5), f(0))
    >> > >
    >> > > This returns TRUE, when it should return FALSE; I think it’s hard to
    >> > make the case that f(5) and f(0) are “approximately equal” in any
    >> > meaningful sense. Digging into the code for all.equal(), I see that
    >> > all.equal(f(5), f(0)) results in a call to all.equal.language(f(5), f(0)),
    >> > which only compares the function texts for equality.
    >> > >
    >> > > If it is decided to leave this behavior as-is, then at least it should
    >> > be documented. Currently I cannot find any documentation for all.equal
    >> > applied to functions.
    >> >
    >> > Clearly it should also compare the environments of the two functions,
    >> > then it would see a difference:
    >> >
    >> >  > all.equal(environment(f(5)), environment(f(0)))
    >> > [1] "Component “x”: Mean relative difference: 1"
    >> >
    >> > Changing the first few lines from
    >> >
    >> >      if (is.language(target) || is.function(target))
    >> >          return(all.equal.language(target, current, ...))
    >> >
    >> > to
    >> >
    >> >      if (is.function(target)) {
    >> >          msg <- all.equal.language(target, current, ...)
    >> >          if (isTRUE(msg)) {
    >> >              msg <- all.equal.environment(environment(target),
    >> > environment(current), ...)
    >> >              if (is.character(msg))
    >> >                msg <- paste("Environments:", msg)
    >> >          }
    >> >          return(msg)
    >> >      }
    >> >      if (is.language(target))
    >> >          return(all.equal.language(target, current, ...))
    >> >
    >> > would fix it.
    >> >
    >> > Duncan Murdoch
    >> >
    >> > ______________________________________________
    >> > R-devel using r-project.org mailing list
    >> > https://stat.ethz.ch/mailman/listinfo/r-devel

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