hyperledger / caliper

A blockchain benchmark framework to measure performance of multiple blockchain solutions https://wiki.hyperledger.org/display/caliper
https://hyperledger.github.io/caliper/
Apache License 2.0
652 stars 404 forks source link

Caliper report is fixed to 2/1 decimal points and isn't influenced by caliper-report-precision option #1289

Open davidkel opened 2 years ago

davidkel commented 2 years ago

WIthin the report code base, as an example

        resultMap.set('Max Latency (s)', CaliperUtils.millisToSeconds(results.getMaxLatencyForSuccessful()).toFixed(2));
        resultMap.set('Min Latency (s)', CaliperUtils.millisToSeconds(results.getMinLatencyForSuccessful()).toFixed(2));

But there is a configuration entry for reports to alter precision | caliper-report-precision | Precision (significant digits) for the numbers in the report. | Which it doesn't use, but the monitors that report do

precision has a specific meaning and changing the TPS/Latency report to use precision results (ie use toPrecision using this configuration option) in a less useful type of output and numbers are being represented as exponentials as the default config value is 3.

As we have an opportunity in 0.5.0 to change things we should look to what we want to do here.

Options are

  1. Leave as is (not the best option as requests to be able to increase the precision of the tps/latency output has been raised by the community)
  2. Move everything to use precision. If we do this then I think we need to change the precision default
  3. Move everything to fixed. This would means removing the caliper-report-precision option and replacing it with a caliper-report-fixed option instead
  4. Provide a hybrid solution where you can specify either fixed or precision but it will apply to both monitors and tps/latency report (ie not split of fixed for one and precision for another)
aklenik commented 2 years ago

@davidkel I'd say option 2 can cover most use cases. What does the fixed precision means (and its related setting) mean? Integer precision? Isn't that the same as precision=0?

davidkel commented 2 years ago

@aklenik with the precision option then it decides based on number of significant digits so can result in 1e+2 type of output for example whereas fixed fixes the number of decimal places (so toPrecision and toFixed result in different ways to output the same value). The property caliper-report-precision is used in conjunction with the toPrecision call in monitors, but the main report is hard coded to use toFixed with values of 2 and 1.

If we go with option 2, then I think we need to change to default from 3 to 4 or 5 to ensure we can get a reasonable output for larger numbers as the default.

aklenik commented 2 years ago

@davidkel Ah, I get it. Well, as a user, I don't really want to concern myself with such representation nuances. I'd just like to state whether I want to see 123 TPS or 123.6 TPS (or 0.78 s) values in my report.

I think monitors shouldn't bother with such a setting. They don't output values to the user directly, they should operate on the "original" number at all times. Then the reporters (HTML, console, etc.) should take this setting into account if they want to make their users happy :)

So based on your previous example, I'd change my answer to provide fixed precision set by the user. Providing precision settings for different dimensions (s, TPS, %, etc.) can be future work if the community requires it.