Negative weights and index range in DistInt(weights)?

Why OSVVM™? Forums OSVVM Negative weights and index range in DistInt(weights)?

This topic contains 6 replies, has 2 voices, and was last updated by Avatar of Andy Jones Andy Jones 6 years, 2 months ago.

Viewing 7 posts - 1 through 7 (of 7 total)
  • Author
    Posts
  • #426
    Avatar of Andy Jones
    Andy Jones
    Member

    The implementation of RandPType.DistInt(weights) appears neither to check for non-negative weights, nor handle negative weights in any coherent manner. However, I’m not sure how they could be handled coherently.

    Zero weights appear to be handled as expected (exclusion).

    I am not sure how/if negative weights could be useful, but am open to suggestions before recommending that all weights be required (and verified) to be naturals.

    If for any reason the weighting fails, the function silently returns integer’left. Perhaps it should assert a warning (configurable severity via default optional argument?) and return 0 or n-1 instead? It is expected that the function would return a value in the range of 0 to n-1 for n weights. Silently returning integer’left is guaranteed to be out of the expected range, but its effects are dependent upon usage and could be difficult to detect/debug. 

    It would be useful to return results in the index range of the weight vector supplied (constrained by integer_vector to natural range <>). Simple literal vector expressions with positional notation would still be indexed 0 to n-1. Thus, instead of using explicit value-weight pairs and/or exclusions, one could simply use the index range of the weight vector, with zero weights for exclusions in a lot of applications. This would be easier to write and read (review).

    Andy

    #429
    Avatar of Jim Lewis
    Jim Lewis
    Member

    Hi Andy,

    Yikes!

    Above, you have identified two issues:

      1) Handling negative weights, is it fatal or are we tolerant [treat  as 0].

      2) Always return values in the range of 0 to n-1, silently returning integer’left is bad.

    WRT issue 1,  it would be easy to treat negative weights as either a zero or a severity failure.   FAILURE is consistent with other errors in the package, such as DistInt with all weights as 0 or in RandInt with a minimum value that is greater than maximum value. 

    How did you end up with negative weights?  Programming error or normal algorithm behavior?  

    OTOH, if negative weights are not a severity failure, then should all 0’s be a severity failure? If all 0’s occurs, it could issue an error level assert and return a value in the range 0 to n-1. Personally, I prefer failures.  However, if an algorithm is generating negative weights, I am not against the DistInt changing them to 0.  Any opinions?

    WRT issue 2, the final return will be changed to have an report… failure prior to it. Without negative weights, the program will not reach this point.  However, it would be bad if something unexpected caused a similar problem.  With the severity failure, it would also be prudent to change the value to a value in the expected range.

    #430
    Avatar of Andy Jones
    Andy Jones
    Member

    Jim,

    Thanks for your quick response.

    I found the issues while reading the source code.

    I don’t have any uses for negative weights, and I’m not really sure conceptually how they would work anyway, but I’m not sure that others won’t have a use for them. If there is a viable usage model for negative weights, then we should either support it or error out (like we do for all-zero weights).

    The other (3rd?) issue I brought up was that it would be useful to return a result within the index range of the Weight array supplied, rather than always returning 0 to ‘length – 1.

    I think it would add useful utility/flexibility, and would not cause problems with most existing code. It would simplify many use cases that currently require supplying value-weight pairs, and would allow use of “others” in those cases too.

    Example:
    constant weighting : integer_vector(8 to 15) := (9 => 25, 13 => 51, others => 4);
    variable value : natural range weighting’range;

    value := myRnd.DistInt(weighting); — returns value in weighting’range

    Andy

    #431
    Avatar of Jim Lewis
    Jim Lewis
    Member

    Hi Andy,

    Issue 3:  return a value in the range of the weight vector

    This would not impact any of the use models I have looked at as I typically have use positional aggregates.  If I remember correctly, named aggregates of arrays always follow the direction of the index base type (natural), and hence, default to a range of “to”.  

    It would mean rewriting the input processing algorithm to handle either a “to” or a “downto” range, but that should be do able.  

    If a tool bug were to mess up the range of an aggregate literal, it would be interesting debugging.  That should not drive the implementation though.

    Also see DistValInt.  This addresses the general case of what you are thinking of above.  Name is different due to lack of introspection of composites.   

    Best,

    Jim

    #432
    Avatar of Andy Jones
    Andy Jones
    Member

    I think if you use indices of ‘low & ‘high instead of ‘left & ‘right, and of ‘low + 1 instead of 1, then the direction of the supplied array will not matter. The actual left-right direction with which you build the DistArray is irrelevant, so long as you search it in the same direction.

    You’re right, DistValInt() will let me do what I want, just not nearly as cleanly. DistValInt() is best for getting random members from a set of significantly non-contiguous integers, where an array with a big enough index range would be too big, and have lots of zero weights (even if it is easy to specify). A good example would be a set of prime numbers.

    BTW, thank you very much for your work on this library!

    Andy

    #546
    Avatar of Jim Lewis
    Jim Lewis
    Member

    Hi Andy,

    Just posted release 2013.04.  In it I addressed the issues with DistInt. 

    1) Negative weights

    Negative weights now generate failure.  If someone continues the simulation, they return Weight’low.  I also handled roll over of the weight sum in the same manner (same code too).

    2) Always return values in the range of 0 to n-1, silently returning integer’left is bad.

    Done. 

    3) return a value in the range of the weight vector

    Done.  Hope this does not break anyone’s code..  I had some code to adjust in CoveragePkg – for the time being, I fixed the code so it would work with any version of RandomPkg, but I plan on changing it in the future to take advantage of this change. 

    Jim

    #582
    Avatar of Andy Jones
    Andy Jones
    Member

    Jim,

    Very nice!

    Thanks,

    Andy

Viewing 7 posts - 1 through 7 (of 7 total)

You must be logged in to reply to this topic.