Re: Design flaw in Rest_collision

[address@hidden]

On 7 nov. 2012, at 09:10, Keith OHara <address@hidden> wrote:

> On Mon, 05 Nov 2012 21:52:15 -0800, address@hidden <address@hidden> wrote:
> 
>> On 6 nov. 2012, at 04:51, Keith OHara <address@hidden> wrote:
>> 
>>> Just to be clear, rest-collision.cc breaks the circular dependency by 
>>> setting
>>> positioning-done := true,
>> 
>> I'm still not sure how this breaks the dependency.  If I look up the extent 
>> of a rest, this will set 'Y-extent to 'calculation-in-progress for the rest. 
>>  Then, when 'Y-extent is read again in F, it will still be 
>> 'calculation-in-progress.  Setting 'positioning-done to #t only prevents 
>> calc_positioning_done by being called multiple times (say by other rests) - 
>> it doesn't fill in the 'calculation-in-progress for 'Y-extent.  Or am I 
>> missing something?
>> 
> 
> Setting 'positioning-done to #t prevents calc_positioning_done() being called 
> repeatedly for the same rest in an endless cycle; that is, it breaks the 
> cycle of dependency.
> 

Yes, here I agree.

> If the process begins with a lookup of Y-extent, then I follow what you say 
> on how the chain of dependencies could lookup the same Y-extent once again, 
> before reaching the end of the data-dependency chain.  The first lookup of 
> Y-extent sets a trap intended to detect and stop cyclic dependencies, while 
> the second lookup activates the trap.
> 

Exactly.

> In this case there would be no further lookups of Y-extent on the same rest, 
> so we could consider this a false-positive activation of the trap.  It is 
> probably wisest to keep the simple trap, even if it can have false-positives. 
> (I argued before that the calculation-in-progress message should be a 
> warning, not error.)
> 
> Do you have input that generates the "calculation-in-progress encountered..." 
> message for rests?
> 

No, but I can make it happen fairly easily from the C++ backend.  I'll be 
posting a patch set later in the week where you'll see an exception that covers 
this case (there are none other like it in the code base).

> Maybe you can arrange to have Rest_collision::force_shift_callback_rest() 
> called before looking up the Y-extent of a rest.
> 
> Or, maybe we should change calc_positioning_done() to call directly 
> Rest::generic_extent_callback() rather than going through the symbol lookup.  
> This way, the C code would assume responsibility for preventing endless data 
> dependency chains.  Your earlier suggestion about having 
> Rest::generic_extent_callback() consistently ignore ledgers should work fine.

This is a good idea except that it'd fail for custom rest stencils.  It'd be 
great if we could drum up a generic solution.

Cheers,
MS