SciProgCentre / plotly.kt

An interactive Kotlin wrapper for plotly visualization tools
https://sciprogcentre.github.io/plotly.kt/
Apache License 2.0
148 stars 21 forks source link

Updating multiple traces in live server is applying only to the first trace #51

Closed zakhenry closed 3 years ago

zakhenry commented 3 years ago

Steps to reproduce:

Create multiple traces, and in dynamic server mode try to update the values of each trace. All values appear to be assigned to the first trace.

I've narrowed this down to Plot.collectUpdates

fun Plot.collectUpdates(plotId: String, scope: CoroutineScope, updateInterval: Long): Flow<Update> {
    return config.flowChanges(scope, updateInterval).flatMapMerge { change ->
        flow<Update> {
            change["layout"].node?.let { emit(Update.Layout(plotId, it)) }
            change.getIndexed("data").values.mapNotNull { it.node }.forEachIndexed { index, metaItem ->
                emit(Update.Trace(plotId, index, metaItem))
            }
        }
    }

The index param that is passed to Update.Trace is always 0. This appears to be because there needs to be another inner loop like metaItem.getIndexed to actually iterate through the traces.

The result of this is evident when inspecting the websocket messages, as I can see the json message field "trace" is always 0, despite having the data for each one of the traces that I am updating.

I must apologise, I'm very new to Kotlin so I'm not really sure how to raise a PR that I can test locally to verify my fix.

altavir commented 3 years ago

I can't reproduce the problem. I've added the second trace to dynamicServer example here and it seems to behave correctly. Could you send me the whole example you are trying to run?

zakhenry commented 3 years ago

Hi @altavir thanks for your speedy response, here is a repro of the issue I'm seeing

import hep.dataforge.meta.invoke
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import kotlinx.html.a
import kotlinx.html.h1
import kscience.plotly.Plotly
import kscience.plotly.models.Bar
import kscience.plotly.plot
import kscience.plotly.server.close
import kscience.plotly.server.pushUpdates
import kscience.plotly.server.serve
import kscience.plotly.server.show
import kotlin.random.Random

fun main() {

    val initialValue = listOf(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)

    val tracesList = arrayOfNulls<Bar>(3).mapIndexed { i, bar ->
        Bar {
            x.strings = initialValue.map { "Column: ${it}" }
            y.numbers = initialValue
            name = "Series ${i}"
        }
    }.associateBy { it.name }

    val server = Plotly.serve(port = 3872) {

        //root level plots go to default page
        page { plotly ->
            h1 { +"This is the plot page" }
            a("/other") { +"The other page" }
            plot(renderer = plotly) {
                traces(tracesList.values)
                layout {
                    title = "Other dynamic plot"
                    xaxis.title = "x axis name"
                    yaxis.title = "y axis name"
                }
            }
        }

        pushUpdates(100)       // start sending updates via websocket to the front-end
    }

    server.show()

    //Start pushing updates
    GlobalScope.launch {

        delay(1000)

        while (isActive) {

            repeat(10) { columnIndex ->

                repeat(3) { seriesIndex ->

                    delay(300)

                    tracesList["Series ${seriesIndex}"]?.let {

                        println("Updating ${it.name}, Column ${columnIndex}")

                        it.y.numbers = it.y.numbers.mapIndexed { i, currentValue ->
                            if (columnIndex == i) {
                                Random.nextInt(0, 100)
                            } else {
                                currentValue
                            }
                        }

                    }

                }

            }

        }
    }

    println("Press Enter to close server")
    readLine()

    server.close()
}

In this example, 3 series are created each with 10 initial bars. In the collect we iterate through both the series and the bars one by one and update the value to a random value. I would expect all bars to eventually have a random value, but the bars that are not in series 0 never update from their initial value

Note that I have my traces (Bars in this case) in a Map<String, Bar> as in my actual application the order in which the results are collected is completely unpredictable, so I need to actually look up which series to update

altavir commented 3 years ago

I think I've reproduced the problem. I will try to understand what happens and how to fix it. The optimized example is placed here: https://github.com/mipt-npm/plotly.kt/blob/dev/examples/src/main/kotlin/misc/dynamicBars.kt

altavir commented 3 years ago

OK, I've finally nailed it. It was a really silly mistake and that is why it took so much time to catch. The problem is that in the method you've shown I use forEachIndexed, which catches the number of the change in list of trace changes, not the trace index. 🤦

I've fixed it. And it will work correctly in the next release (should be in a week or so). image

altavir commented 3 years ago

Thank you very much for the report.

zakhenry commented 3 years ago

Hey that's awesome, thanks for the responsiveness

zakhenry commented 3 years ago

@altavir is there any chance this fix could be applied to 0.2? It doesn't seem to need to be bundled with the 0.3 changes I believe?

altavir commented 3 years ago

Yes, it is pretty simple, but 0.3 is coming in a few days since 1.4.20 is out and I am pressed for time right now. So sadly I do not have time for a separate release activity.

zakhenry commented 3 years ago

Fix verified against 0.3.0-dev-rc 😁

altavir commented 3 years ago

@zakhenry Wow, I just started to write to you.

zakhenry commented 3 years ago

Haha, i was literally just rebasing my plotly branch and got the notification from bintray, great timing!

altavir commented 3 years ago

The release on bintray will be there in several days after I check that the base DataForge library behaves correctly on 1.4.20 in all other libraries.