Comments (8)
Vico 1.14.0 Beta 1 also includes the aforementioned improvement.
from vico.
Hello! Thanks for the report. The chart data is relevant here. Could you please share a set of x and y values for which the crash occurs? You could, for example, update your code to send a log message containing the x and y values before setEntries
is invoked.
from vico.
I struggle to reproduce it right now. I'll post the entries once, I get the StackOverflowError
again.
Instead I get this error, even though there are no duplicate values:
FATAL EXCEPTION: DefaultDispatcher-worker-1
Process: redacted, PID: 20412
java.lang.IllegalArgumentException: The precision of the x values is too large. The maximum is two decimal places.
at com.patrykandpatryk.vico.core.entry.ChartEntryExtensionsKt.calculateXGcd(ChartEntryExtensions.kt:94)
at com.patrykandpatryk.vico.core.entry.ChartEntryModelProducer.getInternalModel(ChartEntryModelProducer.kt:151)
at com.patrykandpatryk.vico.core.entry.ChartEntryModelProducer.getInternalModel$default(ChartEntryModelProducer.kt:133)
at com.patrykandpatryk.vico.core.entry.ChartEntryModelProducer.getModel(ChartEntryModelProducer.kt:158)
at com.patrykandpatryk.vico.core.entry.ChartEntryModelProducer$UpdateReceiver.handleUpdate(ChartEntryModelProducer.kt:213)
at com.patrykandpatryk.vico.core.entry.ChartEntryModelProducer.registerForUpdates(ChartEntryModelProducer.kt:190)
at com.patrykandpatryk.vico.compose.chart.entry.ChartEntryModelExtensionsKt$collectAsState$1$1.invokeSuspend(ChartEntryModelExtensions.kt:116)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [androidx.compose.ui.platform.MotionDurationScaleImpl@e1a8da, androidx.compose.runtime.BroadcastFrameClock@423810b, StandaloneCoroutine{Cancelling}@df17be8, Dispatchers.Default]
Here are the values I put in setEntries()
:
precision-too-large-exception.txt
I suspect a concurrency problem, but don't know how to debug it further.
from vico.
I wrote a test-case with the FloatEntries
from my last comment. (I copied the internal implementation from Vico files, so I could test them):
ChartEntryExtensionsTest.kt.txt
There seems to be a bug in calculateXGcd()
: The last FloatEntry
causes the exception to incorrectly being thrown. If you comment it out, no exception is thrown. The x-value of that entry is unique up to the first three digits (864xx.x) which should be enough?
Edit after some tinkering around with the Vico-code:
I think the problem is in this function (I added a println
):
fun Iterable<Iterable<ChartEntry>>.calculateXGcd() =
flatten().zipWithNext { firstEntry, secondEntry -> abs(secondEntry.x - firstEntry.x) }
.fold<Float, Float?>(null) { gcd, delta ->
println("GCD between $gcd and $delta: ${gcd?.gcdWith(delta) ?: delta}")
gcd?.gcdWith(delta) ?: delta }
?.also { require(it != 0f) { "The precision of the x values is too large. The maximum is two decimal places." } }
?: 1f
The issue is the abs(secondEntry.x - firstEntry.x)
code. Float
s are not precise enough to not introduce errors when interacting like this. e.g. 74176.98f - 46336.89f == 27840.086f
but should be 27840.09
.
Here's a test, which fails:
// this fails!
@Test
fun `float subtract test`() = runTest {
assertEquals("should be 27840.09", 27840.09f, 74176.98f - 46336.89f)
}
By introducing a third digit right of the decimal point, the GCD between 0.01 and 27840.086 is now 0.001 and used as 0f
in the next iteration, when it should still be 0.01f
.
So the solution might be to round after the abs()
-call like this abs(secondEntry.x - firstEntry.x).round(2)
?
Also, I think the output of the println()
should print decreasing GCDs(?), but due to the imprecision it reaches 0f
sometimes, and is replaced in the next iteration with the new delta
, which is (usually) higher again. Only if the last delta
produces 0f
, the Exception is correctly thrown.
Edit 2: Rounding the abs()
call is not correct. It would let the following test fail, which currently succeeds:
@Test
fun `test xcheck fails`() = runTest {
assertThrows<IllegalArgumentException> {
val series: List<List<ChartEntry>> = listOf(
listOf(
FloatEntry(x = 0.0f, y = 1.460994f),
FloatEntry(x = 0.001f, y = 11.586849f),
FloatEntry(x = 0.01f, y = 11.362893f),
)
)
series.calculateXGcd()
}
}
I think the safest thing to do is to only use whole numbers (Int
s or Long
s) for the x-values. Then it should be possible to find the correct GCD. Already the first cast to Float
in entryOf()
introduces precision errors.
Alternatively, ditch the GCD search and trust the Vico-user to give Vico valid Entries?
from vico.
Thanks for looking into this, @littleant! It’s not necessarily a deal-breaker that the first proposed solution causes the last test to fail. The GCD algorithm is in place not to detect excessive precision, but rather to determine the default x step. The zero check is present because an x step of zero would cause unexpected behavior. It happens to be the case that calculateXGcd
produces such an x step for overly precise x values. Such x values can cause other issues too, and there already are instances in which they go undetected—perhaps a dedicated precision check should be added.
The round(2)
solution could be added if it fixed the delta calculation. Unfortunately, from what I can tell, it doesn’t necessarily produce the desired result. While it works for 74176.98f - 46336.89f
, it fails for 174176.98f - 146336.83f
, where the result would have to be rounded down to be correct.
It does look like something has to be changed here. Since, as described above, calculateXGcd
determines the default x step, removing the GCD search would constitute a regression. More specifically, the default scaling of charts would worsen. This isn’t optimal, but it does seem like less of an inconvenience than switching to Int
s or Long
s, which would make it impossible to use fractional x values, forcing Vico users to do more data processing. If calculateXGcd
is removed, it might be possible to add it back once we switch from Float
s to Double
s, which we plan on doing. I’ll discuss the matter with @Gowsky.
In the meantime, I suggest that you multiply your x values by 10 (for one decimal place) or, if that doesn’t help, by 100 (for whole numbers). You can divide them during formatting to have the right values displayed along x-axes.
from vico.
A dedicated precision check would be a solution I think. It would assert that there are no values with a precision lower than 0.01. Afterwards you can define the minimum GCD to be 0.01, regardless of what you detect in calculateXGcd()
.
I agree, round(2)
is not the solution here.
I think switching to Double
would not solve the issue. It would just allow larger values until the same issue surfaces again. (You are just switching from 32bit to 64bit.)
Unless you keep the external API to only use Float
s and immediately convert them to Double
s for internal use (e.g. in entryOf()
. This may work, I don't know.
If you don't want to switch to Int
or Long
in the external API, you could convert to Long
s in the internal API, since you already restrict the precision. e.g. with this conversion logic:
- If x-value is of type
Float
, cast it toDouble
, multiply it by 100 and cast it toLong
s for internal use. - If x-value is of type
Double
, multiply by 100 and cast it toLong
s. (This could be a problem with higher values) - If x-value is of type
Int
, cast toLong
and multiply by 100. - If x-value is of type
Long
, multiply by 100. (This could also lead to problems, if values nearLong.MAX_VALUE
are being used.)
I have added this logic in the hopes that it prevents exceptions. (I'm not sure if .toInt().toFloat()
actually does anything except hiding fractional digits?):
.setEntries(entries.map { entry ->
FloatEntry(x = (entry.x * 100f).toInt().toFloat(), y = entry.y)
})
from vico.
Thanks for the input, @littleant! Regarding switching to Float
s to Double
s, this problem would certainly reappear for some precision level, but what I had in mind is that Double
s might be precise enough to give us a reasonably high precision level that consistently works. We’d still need a precision check. As can be seen here, with Float
s, even two decimal places can be problematic, and a limit of one decimal place would be a bit too restrictive. About chaining toInt
and toFloat
, this doesn’t do anything beyond removing all decimals. A slightly more concise option is floor(entry.x * 100f)
.
Vico 2.0.0 Alpha 6 introduces an improvement that is helpful here: when you set a custom x step, no GCD calculations are run. Thus, when the GCD algorithm fails due to precision issues, rather than transforming your data, you can simply explicitly specify the x step. For your data, getXStep = { 0.01f }
could be added, and everything should work as expected. Of course, it would be optimal for the GCD algorithm not to fail in the first place. We’ll continue to look into this.
from vico.
As I understand, the StackOverflowError
problem hasn’t reappeared, so I’ll be closing this issue for now. If the crash occurs again, please feel free to reopen the issue. We’ll be sharing updates concerning the switch from Float
to Double
under #623. Cheers!
from vico.
Related Issues (20)
- Marker missing style defined by custom MarkerLabelFormatter HOT 9
- Cannot access CartesianChartHost in my project HOT 2
- Reading a state that was created after the snapshot was taken or in a snapshot that has not yet been applied HOT 6
- Cannot make a LabelFormatter on stacked column chart HOT 11
- Markers only works for the first line series HOT 2
- Crash on changing dataset which contains negative values HOT 3
- Throwing OOM Exception when using Chart in LazyColumn HOT 7
- Marker duplicated HOT 2
- No axisValueOverrider in rememberCandlestickCartesianLayer HOT 4
- Feature Missing - Slide to Position HOT 4
- Text alignment on y-axis label only shown correctly for ALIGN_NORMAL HOT 6
- [View] CartesianChartView always handles touches, even if `isClickable` or `isEnabled` is false HOT 6
- Cannot Round NaN to Float HOT 4
- Crash inside AnimatedVisibility() HOT 4
- ColumnChart with MergeMode.Grouped does not receive callback for all the series added through CartesianMarkerVisibilityListener HOT 2
- TopBottomShader.splitY doesn't do anything HOT 1
- Incorrect floating point rounding in xSpacingMultiplier calculation leads to wrong IllegalStateException HOT 6
- At the end the label of bottom axis is cut off HOT 5
- Couple crashes with `java.util.ConcurrentModificationException` and `java.util.NoSuchElementException: Key null is missing in the map` HOT 8
- Bottom label truncated sometimes HOT 8
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from vico.