On Feb 9, 2011, at 9:22 PM, Han-Wen Nienhuys wrote:
Hey all,
It looks good Han-Wen. I must admit that I'm still having trouble understanding why beam collision should be in quanting rather than before quanting, but your code is very clean, logical, and easy to follow.
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 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). 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 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.
Cheers,
MS
ms-beam-collision.pdf