[v2,2/3] manual: floor(log2(fabs(x))) has rounding errors

  Link: <https://inbox.sourceware.org/libc-alpha/20240305150131.GD3653@qaa.vinc17.org/T/#m3ceecda630012995339bcc5448fee451cf277a8b>
Reported-by: Vincent Lefevre <vincent@vinc17.net>
Suggested-by: Vincent Lefevre <vincent@vinc17.net>
Cc: Morten Welinder <mwelinder@gmail.com>
Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 manual/math.texi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
  

Alejandro Colomar <alx@kernel.org> writes:

> -floating-point value.  If @code{FLT_RADIX} is two, @code{logb} is equal
> -to @code{floor (log2 (fabs (x)))}, except
> -it's probably faster.

> +floating-point value.  If @code{FLT_RADIX} is two, @code{logb (x)} is similar
> +to @code{floor (log2 (fabs (x)))}, except
> +that the latter may give an incorrect integer due to intermediate rounding.

So we're replacing "is equal to" with "is similar to", and replacing
"it's probably faster" (understatement ;) with a note about correctness.

I can't think of a different description that is both more correct and
more succinct, nor do I think it's worth the effort to try to come up
with such.

LGTM.
Reviewed-by: DJ Delorie <dj@redhat.com>

Also, if you could avoid unnecessary reformating, that would help with
reviews.  Thanks!
  

Hi DJ,

On Fri, Mar 29, 2024 at 08:24:48PM -0400, DJ Delorie wrote:
> 
> Alejandro Colomar <alx@kernel.org> writes:
> 
> > -floating-point value.  If @code{FLT_RADIX} is two, @code{logb} is equal
> > -to @code{floor (log2 (fabs (x)))}, except
> > -it's probably faster.
> 
> > +floating-point value.  If @code{FLT_RADIX} is two, @code{logb (x)} is similar
> > +to @code{floor (log2 (fabs (x)))}, except
> > +that the latter may give an incorrect integer due to intermediate rounding.
> 
> So we're replacing "is equal to" with "is similar to", and replacing
> "it's probably faster" (understatement ;) with a note about correctness.
> 
> I can't think of a different description that is both more correct and
> more succinct, nor do I think it's worth the effort to try to come up
> with such.
> 
> LGTM.
> Reviewed-by: DJ Delorie <dj@redhat.com>

Thanks!

> Also, if you could avoid unnecessary reformating, that would help with
> reviews.  Thanks!

Actually, I believe it's the other way around.  I did that because the
patch was already horrible without reformatting, since breaking lines at
the 80-col right margin makes for pretty much unreadable patches.

I transformed the sentence to use semantic newlines, which will make
subsequent editions of the sentence much easier to diff.

Here's the original diff before I started breaking the lines at sensible
points:

	diff --git a/manual/math.texi b/manual/math.texi
	index c54eaebb65..ecb77af94d 100644
	--- a/manual/math.texi
	+++ b/manual/math.texi
	@@ -560,8 +560,9 @@ These functions return the base-2 logarithm of @var{x}.
	 @standardsx{logbfNx, TS 18661-3:2015, math.h}
	 @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
	 These functions extract the exponent of @var{x} and return it as a
	-floating-point value.  If @code{FLT_RADIX} is two, @code{logb} is equal
	-to @code{floor (log2 (fabs (x)))}, except it's probably faster.
	+floating-point value.  If @code{FLT_RADIX} is two, @code{logb (x)} is
	+similar to @code{floor (log2 (fabs (x)))}, except that the latter may
	+give an incorrect integer due to intermediate rounding.
	 
	 If @var{x} is de-normalized, @code{logb} returns the exponent @var{x}
	 would have if it were normalized.  If @var{x} is infinity (positive or

To me, it doesn't look any better, since it also reflows the text, but
at random points, instead of breaking at sensible points as I did.

Here's a small description about semantic newlines that we provide in
man-pages(7):

	$ MANWIDTH=64 man man-pages \
	| sed -n '/Use semantic newlines/,/^$/p';
	   Use semantic newlines
	     In the source of a manual page, new sentences  should  be
	     started on new lines, long sentences should be split into
	     lines  at  clause breaks (commas, semicolons, colons, and
	     so on), and long clauses should be split at phrase bound‐
	     aries.  This convention,  sometimes  known  as  "semantic
	     newlines",  makes it easier to see the effect of patches,
	     which often operate at the level of individual sentences,
	     clauses, or phrases.

For more details, you may also find interesting the commit message of a
recent-ish commit that I applied to that text, which contains a quote
from Brian W. Kernighan, who also recommended that practice:

	commit 6ff6f43d68164f99a8c3fb66f4525d145571310c
	Author: Alejandro Colomar <alx.manpages@gmail.com>
	Date:   Fri Nov 12 22:38:11 2021 +0100

	    man-pages.7: Add phrasal semantic newlines advise
	    
	    Brian W. Kernighan, 1974 [UNIX For Beginners]:
	    
	    [
	    Hints for Preparing Documents
	    
	    Most documents go through several versions
	    (always more than you expected)
	    before they are finally finished.
	    Accordingly,
	    you should do whatever possible
	    to make the job of changing them easy.
	    
	    First,
	    when you do the purely mechanical operations of typing,
	    type so subsequent editing will be easy.
	    Start each sentence on a new line.
	    Make lines short,
	    and break lines at natural places,
	    such as after commas and semicolons,
	    rather than randomly.
	    Since most people change documents
	    by rewriting phrases and
	    adding, deleting and rearranging sentences,
	    these precautions simplify any editing you have to do later.
	    ]
	    
	    He mentioned phrases,
	    and they are indeed commonly the operands of patches
	    (see this patch's changes (the second part) as an example),
	    so they make for a much better breaking point than random
	    within a clause that is too long to fit a line.
	    
	    The downside is that they are more difficult to automatically spot
	    than clause breaks (which tend to have associated punctuation).
	    But we are humans writing patches,
	    not machines,
	    and therefore we should be able to decide and detect them better.
	    
	    Link: <https://rhodesmill.org/brandon/2012/one-sentence-per-line/>
	    Cc: G. Branden Robinson <g.branden.robinson@gmail.com>
	    Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>

	diff --git a/man7/man-pages.7 b/man7/man-pages.7
	index 23015b00a..b52a2260a 100644
	--- a/man7/man-pages.7
	+++ b/man7/man-pages.7
	@@ -640,11 +640,13 @@ .SS Formatting conventions for manual pages describing functions
	 .SS Use semantic newlines
	 In the source of a manual page,
	 new sentences should be started on new lines,
	-and long sentences should be split into lines at clause breaks
	-(commas, semicolons, colons, and so on).
	+long sentences should be split into lines at clause breaks
	+(commas, semicolons, colons, and so on),
	+and long clauses should be split at phrase boundaries.
	 This convention, sometimes known as "semantic newlines",
	 makes it easier to see the effect of patches,
	-which often operate at the level of individual sentences or sentence clauses.
	+which often operate at the level of
	+individual sentences, sentence clauses, or phrases.
	 .\"
	 .SS Formatting conventions (general)
	 Paragraphs should be separated by suitable markers (usually either

I would recommend using that practice in glibc, at least for new
paragraphs (that is, I'm not saying you should reflow every line of
current documentation), and also when existing paragraphs would need to
be reflowed anyway due to a patch.

Have a lovely day!
Alex
  

Whoops; I replied from a wrong address.  Please reply to <@kernel.org>.

On Sat, Mar 30, 2024 at 10:27:19AM +0100, Alejandro Colomar wrote:
> Hi DJ,
> 
> On Fri, Mar 29, 2024 at 08:24:48PM -0400, DJ Delorie wrote:
> > 
> > Alejandro Colomar <alx@kernel.org> writes:
> > 
> > > -floating-point value.  If @code{FLT_RADIX} is two, @code{logb} is equal
> > > -to @code{floor (log2 (fabs (x)))}, except
> > > -it's probably faster.
> > 
> > > +floating-point value.  If @code{FLT_RADIX} is two, @code{logb (x)} is similar
> > > +to @code{floor (log2 (fabs (x)))}, except
> > > +that the latter may give an incorrect integer due to intermediate rounding.
> > 
> > So we're replacing "is equal to" with "is similar to", and replacing
> > "it's probably faster" (understatement ;) with a note about correctness.
> > 
> > I can't think of a different description that is both more correct and
> > more succinct, nor do I think it's worth the effort to try to come up
> > with such.
> > 
> > LGTM.
> > Reviewed-by: DJ Delorie <dj@redhat.com>
> 
> Thanks!
> 
> > Also, if you could avoid unnecessary reformating, that would help with
> > reviews.  Thanks!
> 
> Actually, I believe it's the other way around.  I did that because the
> patch was already horrible without reformatting, since breaking lines at
> the 80-col right margin makes for pretty much unreadable patches.
> 
> I transformed the sentence to use semantic newlines, which will make
> subsequent editions of the sentence much easier to diff.
> 
> Here's the original diff before I started breaking the lines at sensible
> points:
> 
> 	diff --git a/manual/math.texi b/manual/math.texi
> 	index c54eaebb65..ecb77af94d 100644
> 	--- a/manual/math.texi
> 	+++ b/manual/math.texi
> 	@@ -560,8 +560,9 @@ These functions return the base-2 logarithm of @var{x}.
> 	 @standardsx{logbfNx, TS 18661-3:2015, math.h}
> 	 @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> 	 These functions extract the exponent of @var{x} and return it as a
> 	-floating-point value.  If @code{FLT_RADIX} is two, @code{logb} is equal
> 	-to @code{floor (log2 (fabs (x)))}, except it's probably faster.
> 	+floating-point value.  If @code{FLT_RADIX} is two, @code{logb (x)} is
> 	+similar to @code{floor (log2 (fabs (x)))}, except that the latter may
> 	+give an incorrect integer due to intermediate rounding.
> 	 
> 	 If @var{x} is de-normalized, @code{logb} returns the exponent @var{x}
> 	 would have if it were normalized.  If @var{x} is infinity (positive or
> 
> To me, it doesn't look any better, since it also reflows the text, but
> at random points, instead of breaking at sensible points as I did.
> 
> Here's a small description about semantic newlines that we provide in
> man-pages(7):
> 
> 	$ MANWIDTH=64 man man-pages \
> 	| sed -n '/Use semantic newlines/,/^$/p';
> 	   Use semantic newlines
> 	     In the source of a manual page, new sentences  should  be
> 	     started on new lines, long sentences should be split into
> 	     lines  at  clause breaks (commas, semicolons, colons, and
> 	     so on), and long clauses should be split at phrase bound‐
> 	     aries.  This convention,  sometimes  known  as  "semantic
> 	     newlines",  makes it easier to see the effect of patches,
> 	     which often operate at the level of individual sentences,
> 	     clauses, or phrases.
> 
> For more details, you may also find interesting the commit message of a
> recent-ish commit that I applied to that text, which contains a quote
> from Brian W. Kernighan, who also recommended that practice:
> 
> 	commit 6ff6f43d68164f99a8c3fb66f4525d145571310c
> 	Author: Alejandro Colomar <alx.manpages@gmail.com>
> 	Date:   Fri Nov 12 22:38:11 2021 +0100
> 
> 	    man-pages.7: Add phrasal semantic newlines advise
> 	    
> 	    Brian W. Kernighan, 1974 [UNIX For Beginners]:
> 	    
> 	    [
> 	    Hints for Preparing Documents
> 	    
> 	    Most documents go through several versions
> 	    (always more than you expected)
> 	    before they are finally finished.
> 	    Accordingly,
> 	    you should do whatever possible
> 	    to make the job of changing them easy.
> 	    
> 	    First,
> 	    when you do the purely mechanical operations of typing,
> 	    type so subsequent editing will be easy.
> 	    Start each sentence on a new line.
> 	    Make lines short,
> 	    and break lines at natural places,
> 	    such as after commas and semicolons,
> 	    rather than randomly.
> 	    Since most people change documents
> 	    by rewriting phrases and
> 	    adding, deleting and rearranging sentences,
> 	    these precautions simplify any editing you have to do later.
> 	    ]
> 	    
> 	    He mentioned phrases,
> 	    and they are indeed commonly the operands of patches
> 	    (see this patch's changes (the second part) as an example),
> 	    so they make for a much better breaking point than random
> 	    within a clause that is too long to fit a line.
> 	    
> 	    The downside is that they are more difficult to automatically spot
> 	    than clause breaks (which tend to have associated punctuation).
> 	    But we are humans writing patches,
> 	    not machines,
> 	    and therefore we should be able to decide and detect them better.
> 	    
> 	    Link: <https://rhodesmill.org/brandon/2012/one-sentence-per-line/>
> 	    Cc: G. Branden Robinson <g.branden.robinson@gmail.com>
> 	    Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
> 
> 	diff --git a/man7/man-pages.7 b/man7/man-pages.7
> 	index 23015b00a..b52a2260a 100644
> 	--- a/man7/man-pages.7
> 	+++ b/man7/man-pages.7
> 	@@ -640,11 +640,13 @@ .SS Formatting conventions for manual pages describing functions
> 	 .SS Use semantic newlines
> 	 In the source of a manual page,
> 	 new sentences should be started on new lines,
> 	-and long sentences should be split into lines at clause breaks
> 	-(commas, semicolons, colons, and so on).
> 	+long sentences should be split into lines at clause breaks
> 	+(commas, semicolons, colons, and so on),
> 	+and long clauses should be split at phrase boundaries.
> 	 This convention, sometimes known as "semantic newlines",
> 	 makes it easier to see the effect of patches,
> 	-which often operate at the level of individual sentences or sentence clauses.
> 	+which often operate at the level of
> 	+individual sentences, sentence clauses, or phrases.
> 	 .\"
> 	 .SS Formatting conventions (general)
> 	 Paragraphs should be separated by suitable markers (usually either
> 
> I would recommend using that practice in glibc, at least for new
> paragraphs (that is, I'm not saying you should reflow every line of
> current documentation), and also when existing paragraphs would need to
> be reflowed anyway due to a patch.
> 
> Have a lovely day!
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es/>
  

On Mär 29 2024, DJ Delorie wrote:

> Also, if you could avoid unnecessary reformating, that would help with
> reviews.  Thanks!

Try diff-mode or diff-minor-mode.