unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
* bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate.
@ 2024-10-26 16:07 Tomas Volf
  2024-10-26 17:22 ` Ludovic Courtès
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Tomas Volf @ 2024-10-26 16:07 UTC (permalink / raw)
  To: 74031; +Cc: Tomas Volf

The specification mandates reals, but the reference implementation
supports complex numbers.  So as implementation extension, support them
as well.

* module/srfi/srfi-64.scm (within-epsilon): Support complex arguments.
---
Proposal for how to extend test-approximate to handle complex arguments.
However it differs from the original one.  That one expected `error' to be a
real number, and used it for comparing both real parts and imaginary parts.

To me, that seems weird.  I would consider it useful to be able to have
different errors for real and imaginary parts.

However I cannot remember the last time I have used complex numbers, so I am not
sure I am qualified to have an opinion here.  What do other people think?

 module/srfi/srfi-64.scm | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/module/srfi/srfi-64.scm b/module/srfi/srfi-64.scm
index 1f60a72e5..5fc23e28a 100644
--- a/module/srfi/srfi-64.scm
+++ b/module/srfi/srfi-64.scm
@@ -776,8 +776,16 @@ Test whether result of @var{test-expr} matches @var{expected} using

 (define (within-epsilon ε)
   (λ (expected actual)
-    (and (>= actual (- expected ε))
-         (<= actual (+ expected ε)))))
+    (let ((e-r (real-part expected))
+          (e-i (imag-part expected))
+          (a-r (real-part actual))
+          (a-i (imag-part actual))
+          (ε-r (real-part ε))
+          (ε-i (imag-part ε)))
+      (and (>= a-r (- e-r ε-r))
+           (<= a-r (+ e-r ε-r))
+           (>= a-i (- e-i ε-i))
+           (<= a-i (+ e-i ε-i))))))

 (define-syntax %test-approximate
   (λ (x)
@@ -808,6 +816,10 @@ Test whether result of @var{test-expr} matches @var{expected} using
 Test whether result of @var{test-expr} is within @var{error} of
 @var{expected}.

+As implementation extension, complex numbers are supported as well.  It tests
+whether real parts are within @code{(real-part @var{error})}, and imaginary
+parts within @code{(imag-part @var{error})}.
+
 @end defspec")

 (define-syntax %test-error
--
2.46.0





^ permalink raw reply related	[flat|nested] 7+ messages in thread

* bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate.
  2024-10-26 16:07 bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate Tomas Volf
@ 2024-10-26 17:22 ` Ludovic Courtès
  2024-10-26 18:46   ` lloda
  2024-10-26 18:35 ` lloda
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Ludovic Courtès @ 2024-10-26 17:22 UTC (permalink / raw)
  To: Tomas Volf; +Cc: 74031, lloda

Hi,

(Cc: lloda.)

Tomas Volf <~@wolfsden.cz> skribis:

> The specification mandates reals, but the reference implementation
> supports complex numbers.  So as implementation extension, support them
> as well.
>
> * module/srfi/srfi-64.scm (within-epsilon): Support complex arguments.
> ---
> Proposal for how to extend test-approximate to handle complex arguments.
> However it differs from the original one.  That one expected `error' to be a
> real number, and used it for comparing both real parts and imaginary parts.
>
> To me, that seems weird.  I would consider it useful to be able to have
> different errors for real and imaginary parts.
>
> However I cannot remember the last time I have used complex numbers, so I am not
> sure I am qualified to have an opinion here.  What do other people think?

Not sure either.  Daniel, is that what you would expect?

Perhaps we should check the reference implementation?

Ludo’.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate.
  2024-10-26 16:07 bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate Tomas Volf
  2024-10-26 17:22 ` Ludovic Courtès
@ 2024-10-26 18:35 ` lloda
  2024-10-26 20:19 ` bug#74031: [PATCH v2] " Tomas Volf
  2024-10-26 21:06 ` bug#74031: [PATCH v3] " Tomas Volf
  3 siblings, 0 replies; 7+ messages in thread
From: lloda @ 2024-10-26 18:35 UTC (permalink / raw)
  To: Tomas Volf; +Cc: 74031


Like I wrote in a separate message, I think test-approximate should check the https://en.wikipedia.org/wiki/Euclidean_distance :

(<= (magnitude (- expected value)) epsilon)

For real numbers, it means the same as the current test. It would also work for other types for which the user has defined - and magnitude, like vectors.

I just checked the old impl:

-(define (%test-approximate= error)
-  (lambda (value expected)
-    (let ((rval (real-part value))
-          (ival (imag-part value))
-          (rexp (real-part expected))
-          (iexp (imag-part expected)))
-      (and (>= rval (- rexp error))
-           (>= ival (- iexp error))
-           (<= rval (+ rexp error))
-           (<= ival (+ iexp error))))))

This is still *a* distance (https://en.wikipedia.org/wiki/Chebyshev_distance), and close numbers will be close either way, but speaking as an engineer who uses complex numbers all day, Euclidean distance is the only one I've ever wanted to use.

Regards

  lloda






^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate.
  2024-10-26 17:22 ` Ludovic Courtès
@ 2024-10-26 18:46   ` lloda
  2024-10-26 20:26     ` Tomas Volf
  0 siblings, 1 reply; 7+ messages in thread
From: lloda @ 2024-10-26 18:46 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 74031, Tomas Volf


> On 26 Oct 2024, at 19:22, Ludovic Courtès <ludo@gnu.org> wrote:
> 
> Hi,
> 
> (Cc: lloda.)
> 
> Tomas Volf <~@wolfsden.cz> skribis:
> 
>> The specification mandates reals, but the reference implementation
>> supports complex numbers.  So as implementation extension, support them
>> as well.
>> 
>> * module/srfi/srfi-64.scm (within-epsilon): Support complex arguments.
>> ---
>> Proposal for how to extend test-approximate to handle complex arguments.
>> However it differs from the original one.  That one expected `error' to be a
>> real number, and used it for comparing both real parts and imaginary parts.
>> 
>> To me, that seems weird.  I would consider it useful to be able to have
>> different errors for real and imaginary parts.
>> 
>> However I cannot remember the last time I have used complex numbers, so I am not
>> sure I am qualified to have an opinion here.  What do other people think?
> 
> Not sure either.  Daniel, is that what you would expect?
> 
> Perhaps we should check the reference implementation?
> 
> Ludo’.

Sorry, I didn't notice this. I replied on another message, but to be clear, the expected error should always be a real number, no matter what you're comparing. If one wants to have separate errors for real and imaginary parts, then one can simply use test-approximate on the real and imaginary parts separately.

Regards 

  Daniel






^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#74031: [PATCH v2] srfi-64: Accept complex numbers in test-approximate.
  2024-10-26 16:07 bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate Tomas Volf
  2024-10-26 17:22 ` Ludovic Courtès
  2024-10-26 18:35 ` lloda
@ 2024-10-26 20:19 ` Tomas Volf
  2024-10-26 21:06 ` bug#74031: [PATCH v3] " Tomas Volf
  3 siblings, 0 replies; 7+ messages in thread
From: Tomas Volf @ 2024-10-26 20:19 UTC (permalink / raw)
  To: 74031; +Cc: Tomas Volf

The specification mandates reals, but the reference implementation
supports complex numbers.  So as implementation extension, support them
as well.

* module/srfi/srfi-64.scm (within-epsilon): Support complex arguments.
---
Require error to be real number and check using (checks notes) Chebyshev
distance.

 module/srfi/srfi-64.scm | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/module/srfi/srfi-64.scm b/module/srfi/srfi-64.scm
index 1f60a72e5..453296364 100644
--- a/module/srfi/srfi-64.scm
+++ b/module/srfi/srfi-64.scm
@@ -776,8 +776,14 @@ Test whether result of @var{test-expr} matches @var{expected} using

 (define (within-epsilon ε)
   (λ (expected actual)
-    (and (>= actual (- expected ε))
-         (<= actual (+ expected ε)))))
+    (let ((e-r (real-part expected))
+          (e-i (imag-part expected))
+          (a-r (real-part actual))
+          (a-i (imag-part actual)))
+      (and (>= a-r (- e-r ε))
+           (<= a-r (+ e-r ε))
+           (>= a-i (- e-i ε))
+           (<= a-i (+ e-i ε))))))

 (define-syntax %test-approximate
   (λ (x)
@@ -808,6 +814,10 @@ Test whether result of @var{test-expr} matches @var{expected} using
 Test whether result of @var{test-expr} is within @var{error} of
 @var{expected}.

+As implementation extension, complex numbers are supported as well.  It tests
+whether real parts are within @code{(real-part @var{error})}, and imaginary
+parts within @code{(imag-part @var{error})}.
+
 @end defspec")

 (define-syntax %test-error
--
2.46.0





^ permalink raw reply related	[flat|nested] 7+ messages in thread

* bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate.
  2024-10-26 18:46   ` lloda
@ 2024-10-26 20:26     ` Tomas Volf
  0 siblings, 0 replies; 7+ messages in thread
From: Tomas Volf @ 2024-10-26 20:26 UTC (permalink / raw)
  To: lloda; +Cc: 74031, Ludovic Courtès

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

Hello,

lloda <lloda@sarc.name> writes:

>> On 26 Oct 2024, at 19:22, Ludovic Courtès <ludo@gnu.org> wrote:
>> 
>> Hi,
>> 
>> (Cc: lloda.)
>> 
>> Tomas Volf <~@wolfsden.cz> skribis:
>> 
>>> The specification mandates reals, but the reference implementation
>>> supports complex numbers.  So as implementation extension, support them
>>> as well.
>>> 
>>> * module/srfi/srfi-64.scm (within-epsilon): Support complex arguments.
>>> ---
>>> Proposal for how to extend test-approximate to handle complex arguments.
>>> However it differs from the original one.  That one expected `error' to be a
>>> real number, and used it for comparing both real parts and imaginary parts.
>>> 
>>> To me, that seems weird.  I would consider it useful to be able to have
>>> different errors for real and imaginary parts.
>>> 
>>> However I cannot remember the last time I have used complex numbers, so I am not
>>> sure I am qualified to have an opinion here.  What do other people think?
>> 
>> Not sure either.  Daniel, is that what you would expect?
>> 
>> Perhaps we should check the reference implementation?
>> 
>> Ludo’.
>
> Sorry, I didn't notice this. I replied on another message, but to be clear, the
> expected error should always be a real number, no matter what you're
> comparing.

Got it, I have sent v2 using the same logic the original implementation
did (according to your message "Chebyshev distance").  Since we are
doing this for purpose of backwards compatibility, it makes sense to
just restore the original behavior as it was.

> If one wants to have separate errors for real and imaginary parts,
> then one can simply use test-approximate on the real and imaginary parts
> separately.

By the same logic, if one wants to use the standard compliant
test-approximate with complex numbers, one can simply call it on real
and imaginary parts separately. ^_^

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#74031: [PATCH v3] srfi-64: Accept complex numbers in test-approximate.
  2024-10-26 16:07 bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate Tomas Volf
                   ` (2 preceding siblings ...)
  2024-10-26 20:19 ` bug#74031: [PATCH v2] " Tomas Volf
@ 2024-10-26 21:06 ` Tomas Volf
  3 siblings, 0 replies; 7+ messages in thread
From: Tomas Volf @ 2024-10-26 21:06 UTC (permalink / raw)
  To: 74031; +Cc: Tomas Volf

The specification mandates reals, but the reference implementation
supports complex numbers.  So as implementation extension, support them
as well.

* module/srfi/srfi-64.scm (within-epsilon): Support complex arguments.
---
v2: Use the same test logic as the reference implementation.
v3: Also adjust the docstring.

 module/srfi/srfi-64.scm | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/module/srfi/srfi-64.scm b/module/srfi/srfi-64.scm
index 1f60a72e5..0f1b5c117 100644
--- a/module/srfi/srfi-64.scm
+++ b/module/srfi/srfi-64.scm
@@ -776,8 +776,14 @@ Test whether result of @var{test-expr} matches @var{expected} using

 (define (within-epsilon ε)
   (λ (expected actual)
-    (and (>= actual (- expected ε))
-         (<= actual (+ expected ε)))))
+    (let ((e-r (real-part expected))
+          (e-i (imag-part expected))
+          (a-r (real-part actual))
+          (a-i (imag-part actual)))
+      (and (>= a-r (- e-r ε))
+           (<= a-r (+ e-r ε))
+           (>= a-i (- e-i ε))
+           (<= a-i (+ e-i ε))))))

 (define-syntax %test-approximate
   (λ (x)
@@ -808,6 +814,10 @@ Test whether result of @var{test-expr} matches @var{expected} using
 Test whether result of @var{test-expr} is within @var{error} of
 @var{expected}.

+As implementation extension, complex numbers are supported as well.  It tests
+whether both real and imaginary parts of @var{test-expr} are within
+@var{error} of real and imaginary parts of @var{expected}.
+
 @end defspec")

 (define-syntax %test-error
--
2.46.0





^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-10-26 21:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-26 16:07 bug#74031: [PATCH] srfi-64: Accept complex numbers in test-approximate Tomas Volf
2024-10-26 17:22 ` Ludovic Courtès
2024-10-26 18:46   ` lloda
2024-10-26 20:26     ` Tomas Volf
2024-10-26 18:35 ` lloda
2024-10-26 20:19 ` bug#74031: [PATCH v2] " Tomas Volf
2024-10-26 21:06 ` bug#74031: [PATCH v3] " Tomas Volf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).