KoalaPlot / koalaplot-core

Koala Plot is a Compose Multiplatform based charting and plotting library written in Kotlin
https://koalaplot.github.io/
MIT License
382 stars 18 forks source link

VerticalBarChart Crash: Cannot round NaN value. #17

Closed Wavesonics closed 1 year ago

Wavesonics commented 1 year ago

This seems to happen when I have bars with very low values, like 0 or 1? not sure, trying to narrow it down.

Exception in thread "AWT-EventQueue-0" java.lang.IllegalArgumentException: Cannot round NaN value.
    at kotlin.math.MathKt__MathJVMKt.roundToInt(MathJVM.kt:1165)
    at io.github.koalaplot.core.bar.VerticalBarChartKt$VerticalBarChart$4$measure$1.invoke(VerticalBarChart.kt:183)
    at io.github.koalaplot.core.bar.VerticalBarChartKt$VerticalBarChart$4$measure$1.invoke(VerticalBarChart.kt:164)
    at androidx.compose.ui.layout.MeasureScope$layout$1.placeChildren(MeasureScope.kt:70)
gsteckman commented 1 year ago

Can you provide the data you are plotting or sample code that produces the issue?

joreilly commented 1 year ago

Seeing the same issue here....getting for example if only one entry but also in some other cases as well....will try to narrow it down.

UPDATE....see that issue with one entry was fixed in https://github.com/KoalaPlot/koalaplot-core/commit/8b9f4275d7c8a6fb5907fbcb85cb885187dff511

gsteckman commented 1 year ago

Yes, and that fix is the 0.4.0-dev1 build. Please let me know if that also resolves the issues you found and I will close this issue.

joreilly commented 1 year ago

I'm not seeing the issue if only one entry if I use 0.4.0-dev1 but am seeing the crash in other cases. One situation that I think is causing it (and this could be issue on my side) is if data is updated such that number of entries is less than it was previously. Following is what I have for example, there's list of players on left and if I click on player I show details about them (including graph of history) on right.

private fun BarPlot(
    playerHistory: List<PlayerPastHistory>,
    tickPositionState: TickPositionState,
    title: String
) {
    val barChartEntries = produceState<List<BarChartEntry<String, Float>>>(emptyList(), playerHistory) {
        value = barChartEntries(playerHistory)
    }

    ChartLayout(
        modifier = paddingMod,
        title = { ChartTitle(title) }
    ) {

        XYChart<String, Float>(
            xAxisModel = CategoryAxisModel(playerHistory.map { it.seasonName }),
            yAxisModel = LinearAxisModel(
                0f..playerHistory.maxOf { it.totalPoints }.toFloat(),
                minimumMajorTickIncrement = 1f,
                minorTickCount = 0
            ),
            xAxisStyle = rememberAxisStyle(
                tickPosition = tickPositionState.horizontalAxis,
                color = Color.LightGray
            ),
            xAxisLabels = {
                AxisLabel(it, Modifier.padding(top = 2.dp))
            },
            xAxisTitle = { AxisTitle("Season") },
            yAxisStyle = rememberAxisStyle(tickPosition = tickPositionState.verticalAxis),
            yAxisLabels = {
                AxisLabel(it.toString(1), Modifier.absolutePadding(right = 2.dp))
            },
            yAxisTitle = {
                AxisTitle(
                    "Points",
                    modifier = Modifier.rotateVertically(VerticalRotation.COUNTER_CLOCKWISE)
                        .padding(bottom = padding)
                )
            },
            verticalMajorGridLineStyle = null
        ) {

            VerticalBarChart(
                series = listOf(barChartEntries.value),
                bar = { series, _, value ->
                    DefaultVerticalBar(
                        brush = SolidColor(Color.Red),
                        modifier = Modifier.fillMaxWidth(BarWidth),
                    ) {
                        HoverSurface { Text(value.yMax.toString()) }
                    }
                }
            )
        }
    }
}

this is last part of stack trace.

Exception in thread "AWT-EventQueue-0" java.lang.IllegalArgumentException: Cannot round NaN value.
    at kotlin.math.MathKt__MathJVMKt.roundToInt(MathJVM.kt:1165)
    at io.github.koalaplot.core.bar.VerticalBarChartKt$VerticalBarChart$4$measure$1.invoke(VerticalBarChart.kt:185)
    at io.github.koalaplot.core.bar.VerticalBarChartKt$VerticalBarChart$4$measure$1.invoke(VerticalBarChart.kt:166)
joreilly commented 1 year ago

fwiw can be reproduced in Compose for Desktop client in https://github.com/joreilly/FantasyPremierLeague

gsteckman commented 1 year ago

Using FantasyPremierLeague, I have traced the issue to this function in the CategoryAxisModel:

/**
 * Returns the offset of the provided string within this Category axis. Returns NaN if
 * [point] is not a valid category.
 * @param point One of the categories for this axis.
 * @return The offset of the category along the axis, between 0 and 1
 */
override fun computeOffset(point: T): Float {
    val index = categories.indexOf(point)
    return if (index == -1) {
        Float.NaN
    } else {
        ((index + 1).toFloat() / (categories.size + 1).toFloat())
    }
}

I set a breakpoint at the call to ChartLayout in the code you pasted above (main.kt line 317 for the desktop client). What I saw happen was that when you change from a playerHistory with 2 entries to one with 1 entry, barChartEntries will still have 2 values. This is when it crashes because the xAxisModel doesn't have as many entries as the bar chart, and you get a -1 value for index in the above.

When moving from a playerHistory of 1 to a PlayerHistory of 2, I noticed the same thing - first playerHistory would update to a size of 2 while barChartEntries has a size of 1, and then on a 2nd recompose barChartEntries has a size of 2. On the first pass since barChartEntries.size < playerHistory.size, it doesn't crash.

I replaced barChartEntries with the following and it doesn't crash anymore:

val barChartEntries = remember(playerHistory) { mutableStateOf(barChartEntries(playerHistory)) }

I haven't used produceState before, but from the docs it is running the producer on a coroutine. I think what is happening is when BarPlot is called with a new value for playerHistory, produceState launches the producer in a co-routine which is asynchronous with the execution of the rest of BarPlot - that is, BarPlot continues to recompose with an "old" value of barChartEntries. When the producer completes, the new state is reflected in barChartEntries and BarPlot recomposes again, this time with playerHistory and barChartEntries now in synchronized.

joreilly commented 1 year ago

Thanks @gsteckman for looking in to that....and can confirm it's working here now as well with your change.