Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
davidkel opened this issue Mar 29, 2022 · 3 comments
Labels
bug Something isn't working component/core Related to the core code-base

Comments

@davidkel
Copy link
Contributor

davidkel commented Mar 29, 2022

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)
@davidkel davidkel added the bug Something isn't working label Mar 29, 2022
@davidkel davidkel added this to the v0.5.0 milestone Mar 29, 2022
@davidkel davidkel added the component/core Related to the core code-base label Mar 29, 2022
@aklenik
Copy link
Contributor

aklenik commented Mar 29, 2022

@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
Copy link
Contributor Author

@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
Copy link
Contributor

aklenik commented Mar 29, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component/core Related to the core code-base
Projects
None yet
Development

No branches or pull requests

2 participants