Re: Sample implementation of make-ball-generator seems incorrect
Bradley Lucier 10 Feb 2024 00:33 UTC
On 2/9/24 6:46 PM, Linas Vepstas wrote:
> I reviewed the pull request
> https://github.com/scheme-requests-for-implementation/srfi-194/pull/24/files
>
> but was unable to spot the actual fix. The pull req is a mashup of
> whitespace changes, test cases, copyrights, code refactoring. From what
> I can tell, it's not a "functional" change, but rather it is about
> correctly handling invalid arguments or poorly-structured, ambiguous
> arguments, right?
Referring to that commit page, this old code led to failures in (the
new) test-ellipse.scm:
(define (make-ball-generator arg)
(define dim-sizes
(cond
((integer? arg) (make-vector (+ 2 arg) 1.0))
((vector? arg) (vector-append arg (vector 1.0 1.0)))
(else (error "expected argument to either be a number
(dimension), or vector (axis length for the dimensions)"))))
(define N (- (vector-length dim-sizes) 2))
(define sphereg (make-sphere-generator (+ N 2)))
; Create a vector of N+2 values, and drop the last two.
; (The sphere already added one, so we only add one more)
(lambda ()
(vector-map
(lambda (el dim-size _) (* el dim-size))
(sphereg)
dim-sizes
(make-vector N #f))))
The new code does not have these failures.
Looking again at it now, the old code was incorrect at the very least
for vector args if (a) a vector arg is given and (b) any of the elements
of the vector arg are not equal to 1.0.
The existing tests caught neither the error in the 2020 code, nor the
error in the existing code, so I added new statistical tests:
test-ellipsoid for the distribution on the surface of the
ellipse/sphere, and test-ellipse for the test of make-ball-generator.
(I called it test-ellipse because it's only a two-dimensional test.)
The parameters are set so these tests fail about 3 times in a 1,000 even
if the generators are correct.
Brad