fix(sql): correct symlog scale invert to be the inverse of apply#1059
Open
devteamaegis wants to merge 1 commit into
Open
fix(sql): correct symlog scale invert to be the inverse of apply#1059devteamaegis wants to merge 1 commit into
devteamaegis wants to merge 1 commit into
Conversation
The symlog scaleTransform's JS invert computed sign(x)*exp(|x|-C), subtracting the constant inside the exponent. The correct inverse of apply (sign(x)*log1p(|x|)) is sign(x)*(exp(|x|)-C), matching the existing sqlInvert. Previously invert(apply(5)) returned ~2.207 instead of 5, corrupting data coordinates when inverting symlog-scaled pixel positions (e.g. plot brushing/zoom).
domoritz
reviewed
Jun 26, 2026
domoritz
left a comment
domoritz
left a comment
Member
There was a problem hiding this comment.
Thank you for the pull request
|
|
||
| describe('scaleTransform', () => { | ||
| it('symlog invert is the inverse of apply', () => { | ||
| const s = scaleTransform<number>({ type: 'symlog' }); |
Member
There was a problem hiding this comment.
Should we test this across constants?
| invert: x => Math.sign(x) * Math.exp(Math.abs(x) - _), | ||
| invert: x => Math.sign(x) * (Math.exp(Math.abs(x)) - _), | ||
| sqlApply: c => (c = asNode(c), mul(sign(c), ln(add(_, abs(c))))), | ||
| sqlInvert: c => mul(sign(c), sub(exp(abs(c)), _)) |
Member
There was a problem hiding this comment.
Can we test that the sql and js versions are consistent?
Member
|
Let's fix this after #1046 is merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's broken
The symlog scale's JS
invertis not the inverse of itsapply. Withapply(x) = sign(x)·log1p(|x|), the correct inverse issign(x)·(exp(|x|) − C), but the code computedsign(x)·exp(|x| − C)— subtracting the constant inside the exponent. Soinvert(apply(5))returns ≈2.207 instead of 5. This corrupts data coordinates whereverscale.invertis used on a symlog scale, e.g. mapping pixel positions back to data values during brushing/zoom (vgplot .../interactors/util/invert.js).Why it happens
Operator placement error: the constant is subtracted within the exponent (
exp(|x|−C)) instead of after exponentiation (exp(|x|)−C). The siblingsqlInvertalready used the correct form, so the JS and SQL paths disagreed.Fix
invert: x => Math.sign(x) * (Math.exp(Math.abs(x)) - constant), matchingsqlInvert.Test
Added a
symlog invert is the inverse of applytest assertinginvert(apply(x)) ≈ xacross several values. Fails before (≈2.207 vs 5), passes after; full sql suite green (254).