Skip to content

[BUG] MultiLine datetime precision loss #10738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bryevdv opened this issue Nov 28, 2020 · 4 comments · Fixed by #10788
Closed

[BUG] MultiLine datetime precision loss #10738

bryevdv opened this issue Nov 28, 2020 · 4 comments · Fixed by #10788

Comments

@bryevdv
Copy link
Member

bryevdv commented Nov 28, 2020

from bokeh.models import ColumnDataSource
from bokeh.plotting import figure, output_file, show

output_file("line.html")

data = {'x': [[1606075294619, 1606075294646, 1606075294670, 1606075294695]],
       'y': [[1, 2, 3, 6]]}

source = ColumnDataSource(data=data)

fig = figure(height=250, x_axis_type='datetime')
fig.multi_line(xs='x', ys='y', source=source)

show(fig)

Screen Shot 2020-11-28 at 12 51 15 PM

@bryevdv
Copy link
Member Author

bryevdv commented Nov 28, 2020

My initial speculation is that the loss of precision affects any nested arrays case (so probably also patches etc)

@mattpap
Copy link
Contributor

mattpap commented Nov 28, 2020

This is due to RaggedArray using NumberArray internally. This is one of those code paths I mentioned before (in a meeting).

@bryevdv
Copy link
Member Author

bryevdv commented Nov 28, 2020

@mattpap yup, just traced it. FWIW here is a diff that fixes, I expect there is a cleaner solution (I ran in to type issues trying to resolve in a singe RaggedArray class)

diff --git a/bokehjs/src/lib/core/util/ragged_array.ts b/bokehjs/src/lib/core/util/ragged_array.ts
index 2ecf403de..bf97e867d 100644
--- a/bokehjs/src/lib/core/util/ragged_array.ts
+++ b/bokehjs/src/lib/core/util/ragged_array.ts
@@ -1,11 +1,11 @@
-import {NumberArray} from "../types"
 import {equals, Equatable, Comparator} from "./eq"
 import {assert} from "./assert"
+import {NumberArray} from "core/types"

 export class RaggedArray implements Equatable {
   static [Symbol.toStringTag] = "RaggedArray"

-  constructor(readonly offsets: Uint32Array, readonly array: NumberArray) {}
+  constructor(readonly offsets: Uint32Array, readonly array: Float64Array) {}

   [equals](that: this, cmp: Comparator): boolean {
     return cmp.arrays(this.offsets, that.offsets) && cmp.arrays(this.array, that.array)
@@ -16,7 +16,7 @@ export class RaggedArray implements Equatable {
   }

   clone(): RaggedArray {
-    return new RaggedArray(new Uint32Array(this.offsets), new NumberArray(this.array))
+    return new RaggedArray(new Uint32Array(this.offsets), new Float64Array(this.array))
   }

   static from(items: number[][]): RaggedArray {
@@ -28,13 +28,69 @@ export class RaggedArray implements Equatable {
       offsets[i] = offset
       offset += length
     }
-    const array = new NumberArray(offset)
+    const array = new Float64Array(offset)
     for (let i = 0; i < n; i++) {
       array.set(items[i], offsets[i])
     }
     return new RaggedArray(offsets, array)
   }

+  *[Symbol.iterator](): IterableIterator<Float64Array> {
+    const {offsets, length} = this
+    for (let i = 0; i < length; i++) {
+      yield this.array.subarray(offsets[i], offsets[i + 1])
+    }
+  }
+
+  private _check_bounds(i: number): void {
+    assert(0 <= i && i < this.length, `Out of bounds: 0 <= ${i} < ${this.length}`)
+  }
+
+  get(i: number): Float64Array {
+    this._check_bounds(i)
+    const {offsets} = this
+    return this.array.subarray(offsets[i], offsets[i + 1])
+  }
+
+  set(i: number, array: ArrayLike<number>): void {
+    this._check_bounds(i)
+    this.array.set(array, this.offsets[i])
+  }
+}
+
+export class RaggedNumberArray implements Equatable {
+  static [Symbol.toStringTag] = "RaggedNumberArray"
+
+  constructor(readonly offsets: Uint32Array, readonly array: NumberArray) {}
+
+  [equals](that: this, cmp: Comparator): boolean {
+    return cmp.arrays(this.offsets, that.offsets) && cmp.arrays(this.array, that.array)
+  }
+
+  get length(): number {
+    return this.offsets.length
+  }
+
+  clone(): RaggedNumberArray {
+    return new RaggedNumberArray(new Uint32Array(this.offsets), new NumberArray(this.array))
+  }
+
+  static from(items: number[][]): RaggedNumberArray {
+    const n = items.length
+    const offsets = new Uint32Array(n)
+    let offset = 0
+    for (let i = 0; i < n; i++) {
+      const length = items[i].length
+      offsets[i] = offset
+      offset += length
+    }
+    const array = new NumberArray(offset)
+    for (let i = 0; i < n; i++) {
+      array.set(items[i], offsets[i])
+    }
+    return new RaggedNumberArray(offsets, array)
+  }
+
   *[Symbol.iterator](): IterableIterator<NumberArray> {
     const {offsets, length} = this
     for (let i = 0; i < length; i++) {
diff --git a/bokehjs/src/lib/models/glyphs/glyph.ts b/bokehjs/src/lib/models/glyphs/glyph.ts
index ddc8b1c77..70394ae35 100644
--- a/bokehjs/src/lib/models/glyphs/glyph.ts
+++ b/bokehjs/src/lib/models/glyphs/glyph.ts
@@ -9,7 +9,7 @@ import {Model} from "../../model"
 import {Anchor} from "core/enums"
 import {logger} from "core/logging"
 import {Arrayable, Rect, NumberArray, Indices} from "core/types"
-import {RaggedArray} from "core/util/ragged_array"
+import {RaggedArray, RaggedNumberArray} from "core/util/ragged_array"
 import {map, max} from "core/util/arrayable"
 import {values} from "core/util/object"
 import {is_equal} from "core/util/eq"
@@ -331,12 +331,12 @@ export abstract class GlyphView extends View {
     for (const prop of this.model) {
       if (prop instanceof p.BaseCoordinateSpec) {
         const scale = prop.dimension == "x" ? x_scale : y_scale
-        let array = self[`_${prop.attr}`] as NumberArray | RaggedArray
+        let array = self[`_${prop.attr}`] as NumberArray | RaggedArray | RaggedNumberArray
         if (array instanceof RaggedArray) {
           const screen = scale.v_compute(array.array)
-          array = new RaggedArray(array.offsets, screen)
+          array = new RaggedNumberArray(array.offsets, screen)
         } else {
-          array = scale.v_compute(array)
+          array = scale.v_compute(array as NumberArray)
         }
         (this as any)[`s${prop.attr}`] = array
       }
diff --git a/bokehjs/src/lib/models/tools/inspectors/hover_tool.ts b/bokehjs/src/lib/models/tools/inspectors/h
index 21135658c..0ced2a9fd 100644
--- a/bokehjs/src/lib/models/tools/inspectors/hover_tool.ts
+++ b/bokehjs/src/lib/models/tools/inspectors/hover_tool.ts
@@ -31,7 +31,7 @@ import * as styles from "styles/tooltips.css"
 export type TooltipVars = {index: number} & Vars

 export function _nearest_line_hit(i: number, geometry: Geometry,
-    sx: number, sy: number, dx: NumberArray, dy: NumberArray): [[number, number], number] {
+    sx: number, sy: number, dx: NumberArray|Float64Array, dy: NumberArray|Float64Array): [[number, number], n
   const d1 = {x: dx[i], y: dy[i]}
   const d2 = {x: dx[i+1], y: dy[i+1]}

@@ -57,7 +57,7 @@ export function _nearest_line_hit(i: number, geometry: Geometry,
     return [[d2.x, d2.y], i+1]
 }

-export function _line_hit(xs: NumberArray, ys: NumberArray, ind: number): [[number, number], number] {
+export function _line_hit(xs: NumberArray|Float64Array, ys: NumberArray|Float64Array, ind: number): [[number,
   return [[xs[ind], ys[ind]], ind]
 }

If you have any quidance I am happy to continue, or if you want to take from here, just lmk either way.

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants