Negative weights and index range in DistInt(weights)?
November 6, 2012 at 07:17 #426
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).
AndyNovember 6, 2012 at 08:36 #429
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.November 6, 2012 at 10:55 #430
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.
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
AndyNovember 6, 2012 at 11:50 #431
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.
JimNovember 6, 2012 at 13:54 #432
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!
AndyApril 27, 2013 at 17:21 #546May 1, 2013 at 07:23 #582
- You must be logged in to reply to this topic.