Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

[Han-Wen Nienhuys]

On Sat, Apr 11, 2020 at 3:55 PM David Kastrup  wrote:
> >> You haven't understood the scheme.  A grob only contains the properties
> >> seminal to its type.  That's why there is a double indirection.  A full
> >> array of 416 entries is needed per grob _type_, not per grob.
> >
> > I still don't see why this requires an overall overhaul of the
> > internal calling convention. You can have a global registry of symbol
> > => uint16 (numbering all the symbols),
>
> It would be pointless to have a single registry for music properties,
> grob properties, stream event properties, and context properties, thus
> significantly blowing up the second-level indexing tables and making for
> a large number of cache misses.

there are 400 grob properties, and about 800 properties in total
(including music and context). 400 requires 9 bits, 800 requires 10
bits. It doesn't appreciably change the problem size.

> > memoize that uint16 like we memoize the symbol today (I think this is
> > what you suggested already).  Then use a uint16 => value hashtable in
> > the grob/prob/etc. A custom hash implementation can save space by
> > storing the uint16 key rather than the 8 byte SCM symbol.
> >
> > This would be a very conservative, localized change that doesn't
> > require setting up machinery to convert types to vectors,
>
> A vector is cheaper to index than a hashtable.  It makes no sense
> talking aobut "setting up machinery" with regard to a vector while
> taking hash tables for granted.
>
> > and it keeps working if grob types change which grob-interfaces they
> > support halfway through.
>
> I don't think there is any point in discussing this patch based on what
> you surmise about an ultimate solution.


Have a look at https://github.com/hanwen/lilypond/tree/grob-dict

It explores different proposals, all of them with neglible to negative
performance impact.

The last series introduces a 'symid' (a global, memoized uint16
identifier for a property), with the optimized hashtable.

I think you could implement what you want to do try by doing the symid
=> per grob type vector index lookup in Grob::internal_xx_property. I
still don't see what you'd do with the infromation you gain by
reconstructing the call sites. Even if you know the C++ type (Grob vs
Context) in the call site, you still won't know the actual grob type
(Stem vs. NoteHead) in the call sites, which you'd need to do further
memoization.

I looked a bit closer at your call-site reorganization. It introduces
'this' as parameter in many places, which I find ugly, and certainly
is nonstandard idiom in C++. I suggest we proceed only if you can
produce a measurable performance improvement. If that happens, I'd be
happy to help deal with any merge conflicts. Until that time, you can
work on the change by not rebasing it.

> >> Hashing outside of a closed set still requires storing the actual symbol
> >> for corroboration.  Hashing inside of a closed set amounts to the
> >> first-stage lookup in my scheme, except that a small number guaranteed
> >> to be unique comes out.  This lookup can be memoised and will work
> >> permanently as opposed to hashing in a growing set.
> >>
> >> > We could experiment with this without requiring a giant rewrite of the
> >> > source code.
> >>
> >> What you call a "giant rewrite" is basically this patch.  It's a purely
> >> syntactical patch only extending rather than reducing possible
> >> approaches and perfectly compatible with different implementations.  The
> >> work for that patch is already done, the impact is similar to what
> >> happened when changing around the syntax of unsmob a few times ian the
> >> past.  It will allow for prolonged work on possibly different
> >> implementations without having to constantly deal with merge conflicts.
> >>
> >> It does not depend on picking a particular implementation.  You
> >> expressed interest in what I was planning to do with the opportunity
> >> this patch opens, I answered.  I do not, however, see the point in
> >> discussing and dissing (partially) unwritten code based on an
> >> incomplete understanding in a handwaving discussion.
> >
> > I think it is not out of the ordinary to discuss plans invasive plans
> > before implementing them.
>
> But we are not discussing an invasive plan here.  This here is an
> extensive, not an invasive change.  There is no point in dissing
> unwritten code that take no project-wide resources.  The point of time
> to review code is after it is proposed.
>
> > I think this change is invasive because it changes the idiom across
> > the source code, and I disagree that this change is net neutral. C++
> > method calls are a more natural idiom plain function calls.
>
> We are not talking about C++ method calls here.  We are talking about
> macro calls.  Memoisation of any kind can only be implemented at the
> macro call level when diversifying on a string literal since those are
> not available as template arguments, otherwise one could transfer the
> j

Re: What's up with the broken web pages?

[Han-Wen Nienhuys]

If you build the website locally, does it not have the 'b' all over the place?

On Sun, Apr 12, 2020 at 12:29 PM David Kastrup  wrote:
>
>
> We currently have web pages looking like
>
> 
>
>
> There are lots of text of the b'...' variety and obviously the download
> sizes are not indicated (so that thing went wrong as well).
>
> This might be related to some Python2/Python3 problem with regard to the
> broken strings.
>
> Anybody with an idea how the deployment of our web sites works or is
> supposed to work and what to do at least in terms of the broken strings?
>
> --
> David Kastrup



-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: What's up with the broken web pages?

[Joram Noeck]

Definitely a python2/3 issue. While python2 uses byte strings by
default, strings are Unicode strings in python3 and byte strings need to
be decoded first.

Surely, you know that and the question is: which part of the website
creation is not fully ported to python3.


python2 [2.7.17]
>>> 'abc', u'abc', b'abc', 'abc'.encode()
('abc', u'abc', 'abc', 'abc')

python3 [3.6.9]
>>> 'abc', u'abc', b'abc', 'abc'.encode()
('abc', 'abc', b'abc', b'abc')


Best,
Joram

Re: Simplify mf-to-table (issue 549840043 by hanw...@gmail.com)

[hanwenn]

https://codereview.appspot.com/549840043/diff/553830043/scripts/build/mf-to-table.py
File scripts/build/mf-to-table.py (right):

https://codereview.appspot.com/549840043/diff/553830043/scripts/build/mf-to-table.py#newcode22
scripts/build/mf-to-table.py:22: import getopt
On 2020/04/11 16:12:52, hahnjo wrote:
> I think this import can be dropped

Done.

https://codereview.appspot.com/549840043/diff/553830043/scripts/build/mf-to-table.py#newcode165
scripts/build/mf-to-table.py:165: 
On 2020/04/11 16:12:53, hahnjo wrote:
> Regular expressions seem a bit overkill here. What about
> root = os.path.splitext (name)
> global_lisp_nm = root + '.global-lisp'
> char_lisp_nm = root + '.lisp'

Done.

https://codereview.appspot.com/549840043/

Re: Simplify mf-to-table (issue 549840043 by hanw...@gmail.com)

[hanwenn]

updated description

https://codereview.appspot.com/549840043/

Use python3 in website.make (issue 581880043 by hanw...@gmail.com)

[hanwenn]

Reviewers: ,

Message:
commit a17fd9412aa927203ea2cbdda01dd37b03178796
Author: Han-Wen Nienhuys 
Date:   Tue Mar 24 21:19:26 2020 +0100

Use python3 in website.make

https://sourceforge.net/p/testlilyissues/issues/5863
http://codereview.appspot.com/581880043



Description:
Use python3 in website.make

Please review this at https://codereview.appspot.com/581880043/

Affected files (+1, -1 lines):
  M make/website.make


Index: make/website.make
diff --git a/make/website.make b/make/website.make
index 
986294f3cbf6fc1e79b4853b287d75f9ecf14ee6..317f53376a9f40f4753cefc050ad7c6da3ffbe27
 100644
--- a/make/website.make
+++ b/make/website.make
@@ -18,7 +18,7 @@ ifeq ($(WEBSITE_ONLY_BUILD),1)
   dir-htaccess=$(trusted-dir)/website-dir.htaccess
   # grab it from PATH
   TEXI2HTML_PROGRAM=texi2html
-  PYTHON=python
+  PYTHON=python3
   PYTHONPATH=$(TRUSTED_DIR)
 else
   ### for normal git

Re: Remove input/regression/test-output-distance.ly (issue 575850043 by hanw...@gmail.com)

[hanwenn]

commit 8a74a0eb329d17154499441396e15811dca1e639
Author: Han-Wen Nienhuys 
Date:   Fri Mar 20 13:28:03 2020 +0100

Remove input/regression/test-output-distance.ly

The self-test for the output-distance.py script fulfills the same
role
in a more automated fashion.

https://sourceforge.net/p/testlilyissues/issues/5850
http://codereview.appspot.com/575850043



https://codereview.appspot.com/575850043/

Re: scripts/build/scan-mf-deps: script to generate MF dependencies (issue 553700043 by hanw...@gmail.com)

[hanwenn]

commit c2eb8beac769a1bf30de357781601322d70095a6
Author: Han-Wen Nienhuys 
Date:   Sun Mar 15 22:35:28 2020 +0100

mf/GNUmakefile: move logic to scripts

This simplifies the build, and removes some GNU Make specific
idioms.

* The mf2pt invocation is intricate. Use a shell script instead

* Metafont dependency scanning uses advanced GNU Make scripting. Use
the Metafont option -recorder instead.

* Take the .log and .tfm files from the MetaPost run. (We were
compiling all fonts twice.)

* Remove some unused variables and rules

https://sourceforge.net/p/testlilyissues/issues/5844
http://codereview.appspot.com/553700043


https://codereview.appspot.com/553700043/

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

[dak]

On 2020/04/12 10:35:24, hanwenn wrote:
> On Sat, Apr 11, 2020 at 3:55 PM David Kastrup 
wrote:
> > >> You haven't understood the scheme.  A grob only contains the
properties
> > >> seminal to its type.  That's why there is a double indirection. 
A full
> > >> array of 416 entries is needed per grob _type_, not per grob.
> > >
> > > I still don't see why this requires an overall overhaul of the
> > > internal calling convention. You can have a global registry of
symbol
> > > => uint16 (numbering all the symbols),
> >
> > It would be pointless to have a single registry for music
properties,
> > grob properties, stream event properties, and context properties,
thus
> > significantly blowing up the second-level indexing tables and making
for
> > a large number of cache misses.
> 
> there are 400 grob properties, and about 800 properties in total
> (including music and context). 400 requires 9 bits, 800 requires 10
> bits. It doesn't appreciably change the problem size.

Populating arrays densely or sparsely makes a large difference in how
well they fit caches.  However, this is completely irrelevant since this
patch does not, I repeat _not_ enforce any particular implementation. 
All that it does is provide the macros with more complete information
and thus enables implementations that are not crippled down by having
macros pretend to be able to deal with overload resolution.

That you don't feel that you are able to make use of that change does
not mean that you are the only developer who should be allowed to work
on such problems and consequently should be able to block off any
attempt of restructuring the source code in a manner that allows other
developers to more easily experiment with other approaches without
having to constantly resolve rebase conflicts because a design decision
unsuitable for refactoring is kept in place for no good reason.

You'll find that we have had several restructurings of macro call
locations in the past years in order to facilitate refactorings, some
with more, some with less success and permanent impact.  In general, the
developers proposing them had reasonable expectations for the directions
those changes would open for them, and reasonable consideration for the
impact on others.  Please keep in mind that LilyPond, as it stands now,
is a colloborative project.  Accommodating the work of others to
proceed, given reasonable goals and impact, is therefore a necessity.  A
stance that amounts to "I cannot make that work, so we shouldn't move in
a direction where you could try" is not really useful for a
colloborative effort of developers with equal standing.

I had several other proposals of syntax that would provide required
information to the macros in case, but this is the least intrusive one
that retains some flavor of overloading.

https://codereview.appspot.com/573670043/

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

[hanwenn]

On 2020/03/30 12:20:17, dak wrote:
> > > Ugly and a maintenance burden since the code is used twice. 
Anything
> > > wrong with my proposal?
> > 
> > I didn't understand your proposal.
> > 
> > > It does not have duplicate code, makes
> > > define-markup-command compilable (while requiring its toplevel
use) and
> > > provides a way of doing the same consistently for module-specific
rather
> > > than toplevel use.
> > >
> > > It sacrifices, like your proposal, non-toplevel-performance for
the sake
> > > of compilability in Guilev2.  It's just that what the parser then
uses
> > > is in a form that could also be used in a reasonably natural
manner from
> > > Scheme.
> > >
> > > Should I write up a patch doing that?
> > 
> > Yes please.
> > 
> 
> Working on it.  

Any update?

https://codereview.appspot.com/577720043/

Re: Reimplement Scheme_hash_table using linear probing. (issue 559790043 by hanw...@gmail.com)

[hanwenn]

this should probably not be merged, given the inability to demonstrate
speed savings.  But I'm uploading the latest version to not leave bugs
in the public record.

https://codereview.appspot.com/559790043/

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

[dak]

On 2020/04/12 10:59:21, hanwenn wrote:
> On 2020/03/30 12:20:17, dak wrote:
> > > > Ugly and a maintenance burden since the code is used twice. 
Anything
> > > > wrong with my proposal?
> > > 
> > > I didn't understand your proposal.
> > > 
> > > > It does not have duplicate code, makes
> > > > define-markup-command compilable (while requiring its toplevel
use) and
> > > > provides a way of doing the same consistently for
module-specific rather
> > > > than toplevel use.
> > > >
> > > > It sacrifices, like your proposal, non-toplevel-performance for
the sake
> > > > of compilability in Guilev2.  It's just that what the parser
then uses
> > > > is in a form that could also be used in a reasonably natural
manner from
> > > > Scheme.
> > > >
> > > > Should I write up a patch doing that?
> > > 
> > > Yes please.
> > > 
> > 
> > Working on it.  
> 
> Any update?

Sorry, got lost among other stuff.  I have something working in a branch
which I'll rebase and upload presently.  It illustrates the point I was
trying to make though I'll have to admit that this looked more
worthwhile "on paper" so far: I am still not all too clear about how
this would help with the byte compilation situation even though it
cleans up the current situation.  Also, making the call of
(current-module) explicit may help in finding a macro invocation that
delays the actual call until it can actually work.

Also it is more of a sketch, and since the sketch was about
define-markup-command, it starts out reverting your union of
define-markup-command and define-markup-command-list.  Since there is
not much of a point in having different implementations for either, this
is of course something that ultimately needs addressing.

So in short, it's not ready for submission in the current form, but it
certainly spells out what I tried expressing in the above comment.  Even
though I am still fuzzy about whether it will be more helpful in finding
a solution for the byte compilation.

https://codereview.appspot.com/577720043/

slur-configuration: fix size_t conversion warning (issue 545860043 by hanw...@gmail.com)

[lemzwerg--- via Discussions on LilyPond development]

LGTM

https://codereview.appspot.com/545860043/

Re: Simplify mf-to-table (issue 549840043 by hanw...@gmail.com)

[lemzwerg--- via Discussions on LilyPond development]

LGTM (without testing)

https://codereview.appspot.com/549840043/

Re: Use python3 in website.make (issue 581880043 by hanw...@gmail.com)

[lemzwerg--- via Discussions on LilyPond development]

Maybe this can be short-tracked to quickly get rid of the faulty
webpage?

https://codereview.appspot.com/581880043/

Re: Use python3 in website.make (issue 581880043 by hanw...@gmail.com)

[jonas . hahnfeld]

On 2020/04/12 11:46:47, lemzwerg wrote:
> Maybe this can be short-tracked to quickly get rid of the faulty
webpage?

This already is in master, see the commit Han-Wen noted above. The
problem is somewhere else, and I think I know how to solve it
(correctly)...

https://codereview.appspot.com/581880043/

Re: What's up with the broken web pages?

[Jonas Hahnfeld]

Apparently it's also broken in a local 'make doc'. I think I chose the
wrong strategy to fix the build with Python 3.5, see 
https://codereview.appspot.com/563860043 for a proposed fix.

As far as I understand, the website build of lilypond.org doesn't use
the scripts from git directly, but instead has "trusted" versions. So
these may need updating after the above change lands? Alternatively,
whoever is able to can update them now and we can test if the change
also works there.

Jonas

Am Sonntag, den 12.04.2020, 12:36 +0200 schrieb Han-Wen Nienhuys:
> If you build the website locally, does it not have the 'b' all over the place?
> 
> On Sun, Apr 12, 2020 at 12:29 PM David Kastrup <
> d...@gnu.org
> > wrote:
> > 
> > We currently have web pages looking like
> > 
> > 
> > 
> > 
> > There are lots of text of the b'...' variety and obviously the download
> > sizes are not indicated (so that thing went wrong as well).
> > 
> > This might be related to some Python2/Python3 problem with regard to the
> > broken strings.
> > 
> > Anybody with an idea how the deployment of our web sites works or is
> > supposed to work and what to do at least in terms of the broken strings?
> > 
> > --
> > David Kastrup


signature.asc
Description: This is a digitally signed message part

weblinks: Fix encoding of link texts (issue 563860043 by jonas.hahnf...@gmail.com)

[hanwenn]

LGTM 

shortcut push? I can update the scripts afterwards.

https://codereview.appspot.com/563860043/

Shortcut Rational addition if either operand is zero (issue 551690046 by hanw...@gmail.com)

[lemzwerg--- via Discussions on LilyPond development]

LGTM.  Maybe you can add a comment w.r.t. speed improvement.

https://codereview.appspot.com/551690046/

Re: weblinks: Fix encoding of link texts (issue 563860043 by jonas.hahnf...@gmail.com)

[jonas . hahnfeld]

Reviewers: hanwenn,

Message:
On 2020/04/12 12:36:22, hanwenn wrote:
> LGTM 
> 
> shortcut push? I can update the scripts afterwards.

commit 23dbc2090b1f45ae26c7f88af6945bde00cb2ddd
Author: Jonas Hahnfeld 
Date:   Sun Apr 12 14:11:37 2020 +0200

weblinks: Fix encoding of link texts

in staging

Description:
weblinks: Fix encoding of link texts

When switching to Python 3, I added the call of .encode('utf-8') to
fix the build with Python 3.5. This converts all link texts into
binary encoding, enclosed by b'...'. The correct fix is to force
utf-8 encoding for sys.stdout.
See e0c78a4c71 ("Use codecs.open() to decode as utf-8") for more
details on this issue and how the default changes with Python 3.7.

Please review this at https://codereview.appspot.com/563860043/

Affected files (+8, -3 lines):
  M scripts/build/create-weblinks-itexi.py


Index: scripts/build/create-weblinks-itexi.py
diff --git a/scripts/build/create-weblinks-itexi.py 
b/scripts/build/create-weblinks-itexi.py
index 
f05e6f33e484fe0f877ccd11fe9851c3cb48fce7..5ac03a3c07a140437f9c71746351446183bd0c7e
 100644
--- a/scripts/build/create-weblinks-itexi.py
+++ b/scripts/build/create-weblinks-itexi.py
@@ -5,9 +5,14 @@
 """ when being called on lilypond.org, pass it the location of the
 top source dir on the command-line. """
 
-import sys
-import os
+import codecs
 import glob
+import os
+import sys
+
+# Force encoding for stdout, Python up to version 3.7 falls back to
+# ASCII with LANG=C (which the build system exports).
+sys.stdout = codecs.getwriter ('utf8') (sys.stdout.detach ())
 
 ### translation data -- shouldn't be here; see issue
 ### https://sourceforge.net/p/testlilyissues/issues/1050/
@@ -425,7 +430,7 @@ def macroLang(name, lang):
 
 def make_macro(name, string):
 print("@macro", name)
-print(string.encode('utf-8'))
+print(string)
 print("@end macro")
 print("")
 

Re: What's up with the broken web pages?

[David Kastrup]

Joram Noeck  writes:

> Definitely a python2/3 issue. While python2 uses byte strings by
> default, strings are Unicode strings in python3 and byte strings need to
> be decoded first.
>
> Surely, you know that

Uh, no?  While I certainly am a natural at embodying a know-it-all, my
expertise is not universal.

> and the question is: which part of the website creation is not fully
> ported to python3.
>
>
> python2 [2.7.17]
 'abc', u'abc', b'abc', 'abc'.encode()
> ('abc', u'abc', 'abc', 'abc')
>
> python3 [3.6.9]
 'abc', u'abc', b'abc', 'abc'.encode()
> ('abc', 'abc', b'abc', b'abc')

That suggests more of a "which part of the website creation has no
qualms mixing up print forms to be read back into Python and actual uses
of strings in a manner that luckily happens to work in Python2" kind of
diagnosis, though it's completely out of my region of expertise.  Could
be that the designers of Python rather than its users deserve part of
the blame here.  No idea.

-- 
David Kastrup

Re: What's up with the broken web pages?

[Phil Holmes]

I've updated the scripts on the website, but it looks like make website is 
not sensitive to a changed Python file and so does not rebuild the web 
pages.  A possibility to force a rebuild would be to touch VERSION, but that 
would make the website out of step with master.


--
Phil Holmes


- Original Message - 
From: "Jonas Hahnfeld" 

To: "Han-Wen Nienhuys" ; "David Kastrup" 
Cc: "lilypond-devel" 
Sent: Sunday, April 12, 2020 1:22 PM
Subject: Re: What's up with the broken web pages?

Re: What's up with the broken web pages?

[Jonas Hahnfeld]

Am Sonntag, den 12.04.2020, 15:50 +0100 schrieb Phil Holmes:
> I've updated the scripts on the website, but it looks like make website is 
> not sensitive to a changed Python file and so does not rebuild the web 
> pages.  A possibility to force a rebuild would be to touch VERSION, but that 
> would make the website out of step with master.

make should only look at the timestamps, so can you literally just
 $ touch VERSION
without changing the content?


signature.asc
Description: This is a digitally signed message part

Re: What's up with the broken web pages?

[Phil Holmes]

There you go.  Fixed :-)

--
Phil Holmes


- Original Message - 
From: "Jonas Hahnfeld" 
To: "Phil Holmes" ; "Han-Wen Nienhuys" 
; "David Kastrup" 

Cc: "lilypond-devel" 
Sent: Sunday, April 12, 2020 3:57 PM
Subject: Re: What's up with the broken web pages?

Re: What's up with the broken web pages?

[Han-Wen Nienhuys]

I simply remove the output directory. I think we should do this
always, because the full build is cheap, and the incremental build
looks iffy.

On Sun, Apr 12, 2020 at 4:50 PM Phil Holmes  wrote:
>
> I've updated the scripts on the website, but it looks like make website is
> not sensitive to a changed Python file and so does not rebuild the web
> pages.  A possibility to force a rebuild would be to touch VERSION, but that
> would make the website out of step with master.
>
> --
> Phil Holmes
>
>
> - Original Message -
> From: "Jonas Hahnfeld" 
> To: "Han-Wen Nienhuys" ; "David Kastrup" 
> Cc: "lilypond-devel" 
> Sent: Sunday, April 12, 2020 1:22 PM
> Subject: Re: What's up with the broken web pages?
>
>


-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Shortcut Rational addition if either operand is zero (issue 551690046 by hanw...@gmail.com)

[nine . fierce . ballads]

LGTM


https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc
File flower/rational.cc (right):

https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode257
flower/rational.cc:257: if (is_infinity ())
The hazard of reviewing this class is that there's always something to
wonder about.  This time, I wonder about adding +infinity and -infinity
together (in either order).

https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode261
flower/rational.cc:261: else if (r.sign_ == 0)
Is this the only micro-optimization you measured?  Other ideas you might
wish to try:

* move the conditions that read this->sign_ together

* are infinities handled first because they are significantly more
common than zeros?  try moving the branches that test for zero before
the branches that test for infinities

* inline a subset of the shortcuts in rational.hh

In any case, noting your findings in a concise comment would help future
maintainers avoid erasing your gains.

https://codereview.appspot.com/551690046/

Reduce number of calls to Bezier::dir_at_point by 2x (issue 583750044 by hanw...@gmail.com)

[lemzwerg--- via Discussions on LilyPond development]

LGTM

https://codereview.appspot.com/583750044/

Re: Shortcut Rational addition if either operand is zero (issue 551690046 by hanw...@gmail.com)

[hanwenn]

 


https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc
File flower/rational.cc (right):

https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode257
flower/rational.cc:257: if (is_infinity ())
On 2020/04/12 16:55:05, Dan Eble wrote:
> The hazard of reviewing this class is that there's always something to
wonder
> about.  This time, I wonder about adding +infinity and -infinity
together (in
> either order).

yes. I recommend against doing that :-)

https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode261
flower/rational.cc:261: else if (r.sign_ == 0)
On 2020/04/12 16:55:05, Dan Eble wrote:
> Is this the only micro-optimization you measured?  Other ideas you
might wish to
> try:
> 
> * move the conditions that read this->sign_ together
> 
> * are infinities handled first because they are significantly more
common than
> zeros?  try moving the branches that test for zero before the branches
that test
> for infinities
> 
> * inline a subset of the shortcuts in rational.hh
> 
> In any case, noting your findings in a concise comment would help
future
> maintainers avoid erasing your gains.

It's the only one I investigated.  The rational addition shows up
because we add Moments together in many places. Since those generally
have grace == 0, it's doing a lot of unwarranted
multiplication/division.

I'll move the zero check first, because it's more likely than the
infinity check.

https://codereview.appspot.com/551690046/

Re: python error running make doc

[Federico Bruni]

Il giorno sab 11 apr 2020 alle 14:44, Jonas Hahnfeld  
ha scritto:

This is odd, this "syntax" works fine for Python 3. However I see the
mentioned error when pasting the line into Python 2. Could you double-
check that your build indeed uses Python 3 (as is required for 
master)?


I have python3 by default since months, so I'm sure.
After I got this error I used a temporary alias to switch python to 
python2, just to see if the error disappeared. But it did not.


Anyway, 'make distclean' solved the problem, as usual.
I was reluctant to clean everything because 'make doc' takes a lot of 
time.