Re: issue 4174043 ready for review

[Han-Wen Nienhuys]

On Thu, Feb 10, 2011 at 9:54 AM, address@hidden
<address@hidden> wrote:

> I think that the problem with doing the collision avoidance in the quanting
> reveals itself around line 341 of beam-quanting.cc
>   if (collisions_.size ())
>     region_size += 2;
> Here, I get the sense that you are expanding the region size of the quanting
> if there is the possibility of collision.  I feel that this will actually
> work against the goal that you had previously set: namely, to avoid
> unnecessary quants if possible.  By moving the beam out of the collision

On the contrary: avoiding unnecessary quants for normal cases makes it
possible for us to use brute force on exceptional situations like by
knees of collisions.

> region before the quanting (in beam.cc, for example), which would do similar
> stuff to that which you have written around line 151, you would eliminate
> having to expand the region size and you'd be able to catch problems that
> your code doesn't currently catch.  That said, your code does catch a great
> deal of stuff - I ran 10 or so tests against the algorithm and not many of
> them diverged (those that did are attached, prefixed ms or mike for me and
> hw for you).

I only see one page in both pdfs?  If you can send me code as .ly, I
can add them to the my regtest, to have more cases.

> However, those that did diverge do come up in actual rep -
> what worries me is that your code (as I understand it) cannot, by virtue of
> how its made, catch several examples from the contemporary repertoire that I
> threw up on that website (notably the Boulez and Crumb) without increasing

I don't see anything by Crumb.  The boulez example is actually a
straight flag (left), and a beam with normal stem on the left.

> the region size to something very large.
> I still advocate the solution in http://codereview.appspot.com/4131044 for
> all the reasons I mention above: I feel that it actually takes less time by
> never expanding the region size and can catch more results.
> Mostly, I think that having two developers doing more or less the same thing
> is a potential waste of time.  My understanding of Lilypond is many orders
> of magnitude inferior to yours, and I assumed (and am still assuming) that
> your work on this problem is addressing an issue that I fundamentally don't
> get.  My critique above still stands, which I why I am still advocating my
> code, but it could be that with a couple tweaks, your code could address
> those issues as well.

My criticism centers on

+      if ((pressure[UP] > 0) && (pressure[DOWN] > 0))
+        {
+          warning ("Cannot resolve beam collision.");
+          return posns;
+        }

ie. if you are dealing with polyphonic music, where the middle voice
will have notes under and over it, your technique does not work.

That said, there may be place for both techniques: if you could detect
that a note+stem is pushing the beam far out of its original position,
you could add an offset.  This offset adding code could be very dumb,
because the fine points of finding the best solution will be handled
by the beam quanting anyway.

> Cheers,
> MS
> <hw-37robust.pdf>
> <hw-beam-collision.pdf>
> <hw-complex.pdf>
> <hw-full-beam-collision.pdf>
> <mike-beam-collision.png>
> <mike-complex.png>
> <mike-up-down.png>
> <hw-beam-collision-beamcount.pdf>
> <ms-beam-collision-beamcount.pdf>
> <ms-37robust.png>

I only received 2 pdf files, was that intentional?

-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen