Issue 4449065: Gets rid of unnecessary logic in beam.cc. (Closed)
DescriptionGets rid of unnecessary logic in beam.cc.
Patch Set 1 #
MessagesTotal messages: 5
If I understand it correctly, Han-Wen's original collision code in beam.cc was treating intervals as if is_empty checked if they were uninitialized or invalid in some way, whereas in fact, is_empty gets rid of anything where left is greater than right. However, the LEFT and RIGHT values of intervals in collision_free contain y values for the beam position, and thus, the left one will invariably be higher than the right one if the beam has a negative slope. Thus, is_empty was (I think) ruling out any solution with a negative slope. I'm not sure if this is too drastic to address issue 1613 (there may need to be some error checking fit in), but I think that it more or less fixes the problem.
Sign in to reply to this message.
On Apr 28, 2011, at 11:37, mtsolo@gmail.com wrote: > Reviewers: , > > Message: > If I understand it correctly, Han-Wen's original collision code in > beam.cc was treating intervals as if is_empty checked if they were > uninitialized or invalid in some way, whereas in fact, is_empty gets rid > of anything where left is greater than right. However, the LEFT and > RIGHT values of intervals in collision_free contain y values for the > beam position, and thus, the left one will invariably be higher than the > right one if the beam has a negative slope. Thus, is_empty was (I > think) ruling out any solution with a negative slope. > > I'm not sure if this is too drastic to address issue 1613 (there may > need to be some error checking fit in), but I think that it more or less > fixes the problem. > I just ran the regtests - it seems this causes other problems in certain circumstances - it was too drastic. That said, I think the general idea of the approach (rethinking the use of is_empty) is valid. Cheers, MS
Sign in to reply to this message.
On Thu, Apr 28, 2011 at 12:37 PM, <mtsolo@gmail.com> wrote: > Reviewers: , > > Message: > If I understand it correctly, Han-Wen's original collision code in > beam.cc was treating intervals as if is_empty checked if they were > uninitialized or invalid in some way, whereas in fact, is_empty gets rid > of anything where left is greater than right. However, the LEFT and > RIGHT values of intervals in collision_free contain y values for the > beam position, and thus, the left one will invariably be higher than the > right one if the beam has a negative slope. Thus, is_empty was (I > think) ruling out any solution with a negative slope. > > I'm not sure if this is too drastic to address issue 1613 (there may > need to be some error checking fit in), but I think that it more or less > fixes the problem. Please read the code carefully to see what it does, rather than guessing: > - Interval v = > - (!collision_free[DOWN].is_empty()) ? > - collision_free[DOWN] : > - collision_free[UP]; > - > - beam_left_y = point_in_interval (v, 2.0); these are not beam slopes. -- Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Sign in to reply to this message.
On Apr 28, 2011, at 10:26 PM, Han-Wen Nienhuys wrote: > On Thu, Apr 28, 2011 at 12:37 PM, <mtsolo@gmail.com> wrote: >> Reviewers: , >> >> Message: >> If I understand it correctly, Han-Wen's original collision code in >> beam.cc was treating intervals as if is_empty checked if they were >> uninitialized or invalid in some way, whereas in fact, is_empty gets rid >> of anything where left is greater than right. However, the LEFT and >> RIGHT values of intervals in collision_free contain y values for the >> beam position, and thus, the left one will invariably be higher than the >> right one if the beam has a negative slope. Thus, is_empty was (I >> think) ruling out any solution with a negative slope. >> >> I'm not sure if this is too drastic to address issue 1613 (there may >> need to be some error checking fit in), but I think that it more or less >> fixes the problem. > > Please read the code carefully to see what it does, rather than guessing: > >> - Interval v = >> - (!collision_free[DOWN].is_empty()) ? >> - collision_free[DOWN] : >> - collision_free[UP]; >> - >> - beam_left_y = point_in_interval (v, 2.0); > > these are not beam slopes. From my reading of the code, collision_free[DOWN] and collision_free[UP] give tenable left and right y positions for a potential beam. is_empty returns true if the right entry of an interval is greater than the left entry. It thus rejects any collision_free[DOWN] or [UP] where the left entry is greater than the right entry (or, in other words, it rejects anything where the left side is higher than the right side (negative slope)). I'm sorry that I'm not getting what it is, but this is the best I could do by mulling over it and using lots of printf's. If it's not doing this, could you please describe what it is doing? Cheers, MS
Sign in to reply to this message.
On Fri, Apr 29, 2011 at 9:59 AM, mike@apollinemike.com <mike@apollinemike.com> wrote: > On Apr 28, 2011, at 10:26 PM, Han-Wen Nienhuys wrote: > >> On Thu, Apr 28, 2011 at 12:37 PM, <mtsolo@gmail.com> wrote: >>> Reviewers: , >>> >>> Message: >>> If I understand it correctly, Han-Wen's original collision code in >>> beam.cc was treating intervals as if is_empty checked if they were >>> uninitialized or invalid in some way, whereas in fact, is_empty gets rid >>> of anything where left is greater than right. However, the LEFT and >>> RIGHT values of intervals in collision_free contain y values for the >>> beam position, and thus, the left one will invariably be higher than the >>> right one if the beam has a negative slope. Thus, is_empty was (I >>> think) ruling out any solution with a negative slope. >>> >>> I'm not sure if this is too drastic to address issue 1613 (there may >>> need to be some error checking fit in), but I think that it more or less >>> fixes the problem. >> >> Please read the code carefully to see what it does, rather than guessing: >> >>> - Interval v = >>> - (!collision_free[DOWN].is_empty()) ? >>> - collision_free[DOWN] : >>> - collision_free[UP]; >>> - >>> - beam_left_y = point_in_interval (v, 2.0); >> >> these are not beam slopes. > > From my reading of the code, collision_free[DOWN] and collision_free[UP] give tenable left and right y positions for a potential beam. is_empty returns true if the right entry of an interval is greater than the left entry. It thus rejects any collision_free[DOWN] or [UP] where the left entry is greater than the right entry (or, in other words, it rejects anything where the left side is higher than the right side (negative slope)). > the UP and DOWN are vertical. This decides whether to go above or below collisions. the result goes into beam_left_y, so it only refers to left side of the beam. -- Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||
