Skip to content

Conversation

@SihongShen
Copy link
Contributor

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Fixes issue #21099, now axis ticks can show number smaller than 1e-10.

Fixed issues

#21099: log axis ticks not containing value less than 1e-10

Details

Before: What was the problem?

No log axis ticks containing value less than 1e-10

Screenshot 2025-07-16 at 15 45 15

After: How does it behave after the fixing?

Log axis ticks smaller then 1e-10 can be shown on the graph.

Screenshot 2025-07-16 at 15 45 49

Document Info

One of the following should be checked.

  • This PR doesn't relate to document changes
  • The document should be updated later
  • The document changes have been made in apache/echarts-doc#xxx

Misc

ZRender Changes

  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

N.A.

Others

Merging options

  • Please squash the commits into a single one when merging.

Other information

Copy link
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert changes under dist.

src/scale/Log.ts Outdated
Comment on lines 70 to 89
function calculatePrecision(value: number): number {
if (!isFinite(value) || value === 0) {
return 0;
}

const str = value.toString();

// Decimals using scientific notation
if (str.includes('e')) {
const [coefficient, exponent] = str.split('e');
const coefficientDecimals = coefficient.includes('.')
? coefficient.split('.')[1].length
: 0;
const exp = parseInt(exponent, 10);
return exp < 0 ? Math.abs(exp) + coefficientDecimals : coefficientDecimals;
}
// Normal decimals
const decimalPart = str.split('.')[1];
return decimalPart ? decimalPart.length : 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling numberUtil.getPrecision instead.

@Ovilia Ovilia linked an issue Jul 16, 2025 that may be closed by this pull request
@Ovilia Ovilia merged commit cee7a8d into apache:master Jul 17, 2025
1 of 2 checks passed
@Ovilia Ovilia added this to the 6.0.0 milestone Jul 17, 2025

// Fix #21099
const precision = numberUtil.getPrecisionSafe(rawVal) || 0;
let powVal = parseFloat(fixRound(rawVal, precision as number, true));
Copy link
Member

@100pah 100pah Jul 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SihongShen @Ovilia I don't fully understand the purpose of this handling.

If rawVal has the fractional part digits <= 20, rawVal will be equal to powVal, nothing changes.
Otherwise, if the fractional part digits > 20, the number.round method internally clamp it to 20, but that doesn't make sense in this scenario and may lead incorrect results, e.g, (1e-40).toFixed(20) we get 0.

image

From my understanding, the "fix round" applied in src/scale is meant to correct rounding errors introduced by floating-point computation, (such as the well-known case where 0.1 + 0.2 === 0.30000000000000004, when representing and calculating decimal fractional number in binary).

But Math.pow(base, tick.value) shouldn't suffer from such kind of rounding error, because the Log.ts guarantees interval and tick.value to be integers, and the base is typically an integer as well. They can be exactly represented in ieee754 64-bit floating-point number (assume no overflow - this is an separate topic).

Therefore, there is no opportunity for rounding errors to be accumulated beyond Number.EPSILON * Math.pow(2, ieee754_double_exponent(tick.value)) and result in a different result when converted back to a decimal string.
(Correct me if I'me wrong.)

Therefore, I think we should just remove this precision and rounding handling here. Just be:

let powVal = mathPow(base, val);

I tried to fix it in #21120

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] log axis ticks not containing value less than 1e-10

3 participants