1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Wouldn't 90909/909090 be a better solution for the input 0.1/0.01? And what would be the expected output for 0.1/0.1234567 ? Making the second number an integer gives 1234567, and that is already larger than MAX_ENTITY_COUNT. \$\endgroup\$ – Martin R Jun 24 '18 at 14:56
  • \$\begingroup\$ @MartinR In the first case, yes that would be a better fit, but I can't think of an algorithm that works well for arbitrary numbers. In that sense it's actually kind of an interesting problem. I'm not sure I understand the second part of your comment. \$\endgroup\$ – anonymous Jun 25 '18 at 4:05
  • \$\begingroup\$ I am just asking what the expected output would be for input[0].raw = 0.1, input[1] = 0.1234567 and MAX_ENTITY_COUNT = 1000000 (and why). Or, as a simpler example input[0].raw = 0.1, input[1] = 0.1234 and MAX_ENTITY_COUNT = 100. \$\endgroup\$ – Martin R Jun 25 '18 at 5:51
  • \$\begingroup\$ @MartinR The way the code is setup right now it bounds that to a 1:2 ratio, which is obviously wrong. However, given the user input that case has never actually come up and I'm not sure it will. Most of the input's I've seen before have been "human friendly" ratios that you can figure out in you head. \$\endgroup\$ – anonymous Jun 25 '18 at 7:24
  • \$\begingroup\$ However, looking at that input again, the 1:2 ratio is "close enough" that it might be acceptable in practice. The minimum entity count is 1E5 so that also helps things out as well. Let me think on things for a bit and reply back. I suspect for the purposes of the problem having a 1:2 ratio come back for those figures would be considered "correctly packed." \$\endgroup\$ – anonymous Jun 25 '18 at 7:26
2
\$\begingroup\$
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'.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.