trjhtr

This code is part of a simulation and inputs will be positive values less than one and inputs are constrained elsewhere. The goal of the algorithm is to scale the inputs up to much larger numbers, although the total sum is bounded. These represent the number of entities in the simulation.

Conceptually it is best to remember that the value 0.001 can also be expressed as 10^-3 and 4.2*10^-3 is also written as 0.0042. What the algorithm is trying to do is determine the exponent needed such that all of the numbers are positive integer values. Once all of the inputs are integers is then ensures that they are scaled within the bound of the maximum entities.

For example, if the simulation has a limit of MAX_ENTITY_COUNT = 1000000 entities and the user enters input[0].raw = 0.01, and input[1] = 0.1 I would expect to see input[0].count = 90900 and input[1].count = 909000 for a total of 999,900 entities.

private void findEntityCount(List<Inputs> input) 
 // Find the smallest exponent based upon the natural log
 int smallest = Integer.MAX_VALUE;
 for (Inputs entry : input) 
 int exp = (int)Math.log(entry.raw);
 if (exp < smallest) 
 smallest = exp;
 
 

 // Calculate the scaling, note that this is closely related to finding the 
 // mantissa of the input value, but subtracting one from the value helps
 // pack things a bit better 
 long total = 0;
 double scaling = Math.pow(10, Math.abs(smallest) - 1);
 for (Inputs entry : input) 
 entry.count = (long)Math.ceil(entry.raw * scaling);
 total += entry.count;
 

 // Use the initial total to find a multiplier that we can use to adjust
 // the initial counts to the final count for the simulation
 double multiplier = MAX_ENTITY_COUNT / total;
 for (Inputs entry : input) 
 entry.count *= multiplier;
 
 

So far this code appears to be working well and resulting in quantities that are scaled correctly and pack the space fairly well. However, since it is playing around with how floating points are stored I'm looking for feedback that:

1. The approach makes sense (i.e., there isn't a better way to do it that is clearer).




2. What are some pitfalls that I should be aware of.

I haven't seen any issues yet, but generally I've also only seen a limited range of inputs.

double smallest = Double.MAX_VALUE;

This is even larger:

double smallest = Double.POSITIVE_INFINITY;

 if (exp < smallest) 
 smallest = exp;
 

Unless you need to do something else in the body, like remembering an index, this can better be replaced with:

 smallest = Math.min(smallest, exp);

Anti-pattern: The -1 you describe is a 'magic value', which in this case, after a bit of reverse engineering, just divides scaling by 10, like so:

double scaling = Math.pow(10, Math.abs(smallest)) / 10;

This is clearer to casual readers of your code, but still a magic number unless rationalized in the comments.

Pitfall: There is no check if entry.raw is actually not larger than one. If it is, your approach breaks.

Performance / Precision / Confusing: don't calculate base-e log for every number and then base-10 exp (that combination alone is already confusing enough). Just find the very smallest number (not the smallest exponent) and set double scaling = 1.0 / smallest;. To get a number-correct result to your current implementation, because of the base confusion, it would be:

double scaling = 40.287487705590465 / smallest;

Note that this just moves the 'magic number' problem. You need to rationally explain the value here, and not just state that it 'packs things better'.