Menu

#4704 Improve beam count handling with subdivided beams

Verified
push
Enhancement
2016-03-02
2015-12-21
Urs Liska
No

Improve beam count handling with subdivided beams

Subdivision beam count was improved in issue 4355, commit 8fa2d858,
but partially reverted in commit 0382ed88.

  • Before 4355 there was always one beam at the subdivision
  • After 4355 the beam count reflected the metric value of
    the subdivision's location
  • After commit 8fa2d858 the beam count matched baseMoment

The behaviour after 4355 is correct as it gives an immediate
feedback on the metric situation, and is suggested by Gould.
The later revert was to avoid a side-effect, namely the inconsistency
with shortened beams: If the remaining length of the beam is less
than the length of the current metric group the result was
confusing.

This patch reverts the effect of the latest commit, reinstating
the behaviour of issue 4355. However, it improves handling to
remove the unwanted side-effect:
- when the remaining length of the beam is shorter than the
next group (e.g. subdivision at an 1/8 point, but only 3/32
left) the beam count matches the length of the remainder:
1/16 <= remainder < 1/8 => 2 beams
1/32 <= remainder < 1/16 => 3 beams
etc.
- but if only one stem is left after the subdivision this is not
applied. The subdivision has the regular number matching the
metric value, and the trailing single stem has beamlets according
to its actual length

http://codereview.appspot.com/276560043

Discussion

  • Urs Liska

    Urs Liska - 2015-12-22

    Respond to review comments

    http://codereview.appspot.com/276560043

     
  • Anonymous

    Anonymous - 2015-12-22
    • Description has changed:

    Diff:

    
    
    • Needs: -->
    • Patch: new --> review
    • Type: --> Enhancement
     
  • Anonymous

    Anonymous - 2015-12-22

    Passes make, make check and a full make doc.

     
  • Urs Liska

    Urs Liska - 2015-12-23

    Revert changes to snippets file

    http://codereview.appspot.com/276560043

     
  • Urs Liska

    Urs Liska - 2015-12-23

    Simplify code by cleaning up "old" code

    http://codereview.appspot.com/276560043

     
  • Urs Liska

    Urs Liska - 2015-12-23

    Add commit with updated doc snippet

    http://codereview.appspot.com/276560043

     
  • Anonymous

    Anonymous - 2015-12-23
    • Needs: -->
    • Patch: new --> review
    • Type: -->
     
  • Anonymous

    Anonymous - 2015-12-23

    Passes make, make check and a full make doc.

     
  • Anonymous

    Anonymous - 2015-12-25
    • Patch: review --> countdown
    • Type: --> Enhancement
     
  • Anonymous

    Anonymous - 2015-12-25

    Patch on countdown for December 28th.

     
  • Anonymous

    Anonymous - 2015-12-28
    • Patch: countdown --> push
     
  • Anonymous

    Anonymous - 2015-12-28

    Patch counted down - please push.

     
  • Urs Liska

    Urs Liska - 2015-12-28

    Pushed as

    706d797884199982b6b849ba11942865d0b17fd2 4704: Update doc-snippet through makelsr.py

    4b994f8a49892039def08d141daf12c128a12cd4 4704: Update LSR snippet for documentation

    9b8b5c2e05183a82ff687db186eb3eb3ff57f94e 4704: Add regtests and improve wording

    29022d7b0d1d1a52d221def82511d4d393c52e8d 4704: Improve beam subdivision beaming count

    73d4697d0b91c1743e2d3dc31e1c4e87ce03da7b 4704: Partially revert 0382ed88: "Adjust beam subdivision"

     

    Last edit: Urs Liska 2016-01-04
    • David Kastrup

      David Kastrup - 2016-01-03

      Please always include at least the full unabbreviated commit ids, preferably also at least the summary line of the commit message.

      If you don't include the full unabbreviated commit id and a particular commit is found to have caused a problem later on, it becomes much harder finding it in the tracker since nobody has an idea just how many digits of an abbreviated commit id to search for.

      In general, don't invent your own standards without asking back on the developer list: for many established practices there is an underlying reason and rationale that may not be readily apparent.

      Thanks

       
  • Urs Liska

    Urs Liska - 2015-12-28
    • labels: --> Fixed_2_19_35
    • status: Started --> Fixed
     
  • Phil Holmes

    Phil Holmes - 2015-12-29

    Urs,

    I'm doing some LSR tidying, and so I've looked at this patch and snippet. AFAICS, the snippet itself behaves exactly the same with the latest patched binary as it did with 2.18. Is it supposed to illustrate differences caused by your changes?

     
  • Simon Albrecht

    Simon Albrecht - 2016-03-02
    • status: Fixed --> Verified