RobinHankin / stokes

methods for exterior calculus
https://robinhankin.github.io/stokes/
3 stars 0 forks source link

test_alt.R fails #48

Closed RobinHankin closed 3 years ago

RobinHankin commented 3 years ago

automated email from Brian Ripley:

[snip]

  ══ Failed tests ════════════════════════════════════════════════════════════════
  ── Error (test_alt.R:24:9): Function Alt() behaves itself ──────────────────────
  Error: subscript out of bounds
  Backtrace:
       █
    1. └─stokes:::foo(S) test_alt.R:24:8
    2.   ├─stokes::Alt(Alt(S)) test_alt.R:6:8
    3.   │ └─stokes::is.kform(S)
    4.   └─stokes::Alt(S)
    5.     ├─stokes::ktensor(include_perms(consolidate(out))/factorial(ncol(index(out))))
    6.     │ ├─base::stopifnot(is.spray(S))
    7.     │ └─spray::is.spray(S)
    8.     └─stokes::include_perms(consolidate(out))
    9.       └─stokes:::f(index(S)[1, , drop = TRUE], value(S)[1])
   10.         ├─spray::spray(matrix(v[p], nrow(p), k), sgn(p) * x)
   11.         └─base::matrix(v[p], nrow(p), k)

  [ FAIL 1 | WARN 0 | SKIP 0 | PASS 942 ]
  Error: Test failures
  Execution halted
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... OK
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* checking for detritus in the temp directory ... OK
* DONE

Status: 1 ERROR

The problem is triggered as follows:

> dput(out)
structure(list(index = structure(c(4L, 2L, 4L, 3L, 4L, 2L, 2L, 
3L, 3L), .Dim = c(3L, 3L)), value = c(1, 2, 3)), class = c("ktensor", 
"spray"))
> out
A linear map from V^3 to R with V=R^4:
           val
 4 3 2  =    1
 2 4 3  =    2
 4 2 3  =    3
> Alt(out)
Error in index(S)[1, , drop = TRUE] : subscript out of bounds
> 

The problem is triggered in the final line of Alt(), viz ktensor(include_perms(consolidate(out))/factorial(ncol(index(out)))). The issue is that consolidate(out) is the zero spray object (this is because of a peculiar cancellation in the terms above). Function include_perms() does not like it:

> include_perms(0*rspray())
Error in index(S)[1, , drop = TRUE] : subscript out of bounds
> 
RobinHankin commented 3 years ago

The bug can be fixed by adding if(is.zero(S)){return(S)} to the first line of include_perms(). . . . . . but the code is inefficient because the for() loop in include_perms() repeatedly adds spray objects to spray objects, and checking for repeated rows (which is done by the C back-end of spray) takes a long time. It should be possible to use the fact that no repeated rows can possibly occur [because include_perms() is sent the output of consolidate() which guarantees that no row is a permutation of any other row]. We would then build the index and coefficients using ordinary matrix replacement methods.

Even if called directly, we can still streamline the code by returning spray(out) which would coalesce repeated rows but only once.