Re: Fixes slope errors from incorrect X extents in Beam::print. (issue 5293060)

[address@hidden]

On Nov 1, 2011, at 7:59 PM, Keith OHara wrote:

> On Tue, 01 Nov 2011 04:41:22 -0700, address@hidden <address@hidden> wrote:
> 
>> Where is this advertising?  Is it the word prolongation?
> 
> Yes. When a beam grob is passed to a function called prolongation people will 
> at first think the function lengthens the beam.
> 
>> To make things more consistent, I could call the functions 
>> individual-breaking, strict-breaking, and peters-breaking.
> 
> Do these functions break the beam?
> 
> I thought they placed the beam, or a part of a broken beam, with different 
> methods like place-individually  align-with-broken-parts  
> slope-like-broken-parts.
> 

Renamed following your suggestions.

> 
>>> The boolean really means, if 'me' is a segment of a broken beam, then
>>> beam 'me' together with my fellow broken-intos.
>>> Maybe the boolean should be align-broken-segments or something.
>>> 
>> 
>> Not necessarily - in peters-prolongation, this is set to false for one of 
>> the quants even though we are doing broken beaming (to find the quants of 
>> the individual beam).
> 
> That's kind of what confused me.  In a case where the upper-level scheme code 
> is trying to make beam slopes consistent, but not necessarily continuous in 
> height, you tell the lower-level code consistent_broken_slope=0.  I see how 
> it works, but I would have seen it sooner if the variable name were beelzebub.
> 
> My problem is that the boolean consistent_broken_slope doesn't control 
> whether things are consistent, and affects height in addition to slope.  It 
> works at the detailed level, when quanting a (part of a) beam, choosing 
> whether to align him with his fellow broken-intos.  So I renamed it in my 
> mind as align_broken_intos
> 

Renamed following your suggestion.

>> I like do_initial_slope_calculations_ because I think it reads cleaner in 
>> the constructor
> That boolean name is fine. My comment was still talking about the interface 
> to Beam_scoring_problem
> 
>> 
>>> http://codereview.appspot.com/5293060/diff/19014/lily/beam-quanting.cc#newcode743
>>> lily/beam-quanting.cc:743: if (do_initial_slope_calculations_)
>>> Even if we made an earlier pass, and avoid the collisions and/or made a
>>> pretty knee, the averaging in peters-prolungation messed up the results
>>> so we would have to do it again.
>> 
>> Exactly.
>> Should I add this as a comment?
>> 
> Well, my statement was one of confusion, because I say we /would/ need to 
> look harder for good positions, but the code says we do not look in this case.
> A non-confused comment would help.
> 

Sorry - I'm still not quite getting what would help.  Could you please suggest 
a comment to put here?

>>> 
>>> http://codereview.appspot.com/5293060/diff/19014/lily/beam-quanting.cc#newcode989
>>> Does this have something to do with X-extents, or Beam::print, or broken
>>> beams?
>> 
>> Yes - because this used to yield two equal quants, the regtests were 
>> changing with my new code even though the values of the quants didn't.  The 
>> fact that they fell on correct quants before is pure luck (the correct quant 
>> was the first in the pack).  This allows kneed beams to attain the correct 
>> values of the old regtests.
>> 
> That must have been surprising.

'twas

> Sounds like a pre-commit independent of the main commit
> 

'tis possible - I can push as 2 commits.

Cheers,
MS