From 1938625fd3cabaf34f149f72f131748e4607f681 Mon Sep 17 00:00:00 2001 From: Benjamin Schmidt Date: Wed, 16 Oct 2024 13:47:33 -0400 Subject: [PATCH] clean transitions between struct-nested fields (#160) There was a bug caused by a failure to properly sort values in the list of buffers. When delving into struct array fields, an additional sort operation is necessary. This fixes that, and also includes a number of type improvements that I made while hunting down that bug. ---- > [!IMPORTANT] > Fixes sorting issue in `SwitchPositions.svelte` for struct fields and improves type safety with `TupleMap` across the codebase. > > - **Behavior**: > - Fixes sorting issue in `SwitchPositions.svelte` by ensuring nested struct fields are handled correctly. > - Updates `click()` function to handle struct fields with dynamic subfields. > - **Type Improvements**: > - Replaces `Record` with `TupleMap` in `Deeptable.ts`, `AestheticSet.ts`, and `regl_rendering.ts` for better type safety. > - Updates `TupleMap` and `TupleSet` usage in `utilityFunctions.ts`. > - **Misc**: > - Removes unused code and comments in `SwitchPositions.svelte` and `selection.ts`. > - Minor logging and code cleanup in `regl_rendering.ts`. > > This description was created by [Ellipsis](https://www.ellipsis.dev?ref=nomic-ai%2Fdeepscatter&utm_source=github&utm_medium=referral) for 5bcd6a80a14b6058e4ec5872d18c21bd56039ab5. It will automatically update as commits are pushed. --- dev/svelte/SwitchPositions.svelte | 76 +++++++++++++------------------ src/Deeptable.ts | 1 - src/aesthetics/AestheticSet.ts | 6 ++- src/regl_rendering.ts | 67 +++++++++++++++------------ src/selection.ts | 2 - src/types.ts | 2 +- src/utilityFunctions.ts | 2 +- 7 files changed, 76 insertions(+), 80 deletions(-) diff --git a/dev/svelte/SwitchPositions.svelte b/dev/svelte/SwitchPositions.svelte index a5db8d477..eb133374f 100644 --- a/dev/svelte/SwitchPositions.svelte +++ b/dev/svelte/SwitchPositions.svelte @@ -13,75 +13,61 @@ let positionNum = 0; async function click() { - for (let i = 0; i < 10; i++) { - if (scatterplot.deeptable.transformations['struct' + i]) { - continue; - } - scatterplot.deeptable.transformations['struct' + i] = async function ( - tile, - ) { - // Create a nested struct with a change. - const x = (await tile.get_column('x')).toArray(); - const y = (await tile.get_column('y')).toArray(); + if (scatterplot.deeptable.transformations['struct'] === undefined) { + scatterplot.deeptable.transformations['struct'] = async function (tile) { + const fields = []; + const vectors = []; + for (let h = 0; h < 10; h++) { + // Create a nested struct with a change. + const x = (await tile.get_column('x')).toArray(); + const y = (await tile.get_column('y')).toArray(); - const x_ = new Float32Array(x.length); - const y_ = new Float32Array(y.length); + const x_ = new Float32Array(x.length); + const y_ = new Float32Array(y.length); - for (let i = 0; i < x.length; i++) { - const r = (Math.random() + Math.random()) / 3; - const theta = Math.random() * Math.PI * 2; - x_[i] = x[i] + Math.cos(theta) * r; - y_[i] = y[i] + Math.sin(theta) * r; + for (let i = 0; i < x.length; i++) { + const r = (Math.random() + Math.random()) / 3; + const theta = Math.random() * Math.PI * 2; + x_[i] = x[i] + Math.cos(theta) * r; + y_[i] = y[i] + Math.sin(theta) * r; + } + fields.push(new Field(`x${h}`, new Float32())); + vectors.push(vectorFromArray(x_).data[0]); + fields.push(new Field(`y${h}`, new Float32())); + vectors.push(vectorFromArray(y_).data[0]); } const d = makeData({ - type: new Struct([ - new Field('x', new Float32()), - new Field('y', new Float32()), - ]), - children: [vectorFromArray(x_).data[0], vectorFromArray(y_).data[0]], + type: new Struct(fields), + children: vectors, }); const r = new Vector([d]); return r; }; - scatterplot.deeptable.map((d) => d.get_column('struct' + i)); + scatterplot.deeptable.map((d) => d.get_column('struct')); + // await new Promise((resolve) => { + // setTimeout(() => resolve(), 100); + // }); } - await new Promise((resolve) => { - setTimeout(() => resolve()); - }, 100); - let r = 'struct' + (positionNum++ % 10); + positionNum = (positionNum + 1) % 10; await scatterplot.plotAPI({ duration: 1000, encoding: { x: { - field: r, - subfield: ['x'], + field: 'struct', + subfield: [`x${positionNum}`], transform: 'literal', domain: [-10, 10], }, y: { - field: r, - subfield: ['y'], + field: 'struct', + subfield: [`y${positionNum}`], transform: 'literal', domain: [-10, 10], }, }, }); - // await scatterplot.plotAPI({ - // encoding: { - // x: { - // field: scatterplot.prefs.encoding.x.field === 'x' ? 'y' : 'x', - // transform: - // scatterplot.prefs.encoding.x.field === 'x' ? 'linear' : 'literal', - // }, - // y: { - // field: scatterplot.prefs.encoding.y.field === 'y' ? 'x' : 'y', - // transform: - // scatterplot.prefs.encoding.y.field === 'y' ? 'linear' : 'literal', - // }, - // }, - // }); } diff --git a/src/Deeptable.ts b/src/Deeptable.ts index 4d7134a22..23cf11f6c 100644 --- a/src/Deeptable.ts +++ b/src/Deeptable.ts @@ -366,7 +366,6 @@ export class Deeptable { if (dim === undefined) { continue; } - console.log({ dim }); dim = (dim as Field>).type.children.find( (d) => d.name === sub, ); diff --git a/src/aesthetics/AestheticSet.ts b/src/aesthetics/AestheticSet.ts index 20273a172..b742890a3 100644 --- a/src/aesthetics/AestheticSet.ts +++ b/src/aesthetics/AestheticSet.ts @@ -55,7 +55,7 @@ export class AestheticSet { // Used for things like 'what color would this point be?' if (this.store[aesthetic]) { - const v = this.store[aesthetic] + const v = this.store[aesthetic]; return v; } if (!dimensions[aesthetic]) { @@ -84,7 +84,7 @@ export class AestheticSet { } } - _neededFields: TupleSet = new TupleSet(); + _neededFields: TupleSet = new TupleSet(); get neededFields(): string[][] { return [...this._neededFields.values()]; @@ -117,6 +117,8 @@ export class AestheticSet { // Update the needed fields. this._neededFields.clear(); + // We always need the 'ix' column + this._neededFields.add(['ix']); for (const v of Object.values(this.store)) { if (v instanceof StatefulAesthetic) { for (const f of v.neededFields) { diff --git a/src/regl_rendering.ts b/src/regl_rendering.ts index 85c020818..a4cab1789 100644 --- a/src/regl_rendering.ts +++ b/src/regl_rendering.ts @@ -67,7 +67,7 @@ export class ReglRenderer extends Renderer { public reglframe?: REGL.Cancellable; public bufferManager: BufferManager; - private aes_to_buffer_num?: Record; + private aes_to_buffer_num: TupleMap = new TupleMap(); private variable_to_buffer_num?: TupleMap; private buffer_num_to_variable?: string[][]; @@ -188,9 +188,9 @@ export class ReglRenderer extends Renderer { last_webgl_scale: this._webgl_scale_history[1], use_scale_for_tiles: this._use_scale_to_download_tiles, grid_mode: 0, - buffer_num_to_variable: buffer_num_to_variable!, - aes_to_buffer_num: aes_to_buffer_num!, - variable_to_buffer_num: variable_to_buffer_num!, + buffer_num_to_variable: buffer_num_to_variable, + aes_to_buffer_num, + variable_to_buffer_num: variable_to_buffer_num, color_picker_mode: 0, // whether to draw as a color picker. position_interpolation: this.aes.position_interpolation, zoom_matrix: [ @@ -211,7 +211,8 @@ export class ReglRenderer extends Renderer { }; // Clone. - return JSON.parse(JSON.stringify(props)) as DS.GlobalDrawProps; + // return JSON.parse(JSON.stringify(props)) as DS.GlobalDrawProps; + return props; } get default_webgl_scale() { @@ -260,7 +261,6 @@ export class ReglRenderer extends Renderer { a.tile_id ); }); - // console.log({ prop_list }); this._renderer(prop_list); } @@ -537,11 +537,11 @@ export class ReglRenderer extends Renderer { this.regl.clear({ color: [0, 0, 0, 0] }); // read onto the contour vals. this.render_points(props); - this.regl.read(this.contour_vals as Uint8Array); + this.regl.read(this.contour_vals); // Could be done faster on the GPU itself. // But would require writing to float textures, which // can be hard. - v = sum(this.contour_vals as Uint8Array); + v = sum(this.contour_vals); }); return v; } @@ -644,14 +644,14 @@ export class ReglRenderer extends Renderer { this.regl.clear({ color: [0, 0, 0, 0] }); // read onto the contour vals. this.render_points(props); - this.regl.read(this.contour_vals!); + this.regl.read(this.contour_vals); }); // 3-pass blur this.blur(this.fbos.contour, this.fbos.ping, 3); this.fbos.contour.use(() => { - this.regl.read(this.contour_vals!); + this.regl.read(this.contour_vals); }); let i = 0; @@ -840,7 +840,7 @@ export class ReglRenderer extends Renderer { _, { aes_to_buffer_num }: P, ) => { - const val = aes_to_buffer_num[`${dim}--${time}`]; + const val = aes_to_buffer_num.get([dim, time]); if (val === undefined) { return -1; } @@ -884,14 +884,13 @@ export class ReglRenderer extends Renderer { // we run out of buffers, the previous state of the requested aesthetic will just be thrown // away. - type time = 'current' | 'last'; type BufferSummary = { - aesthetic: keyof typeof dimensions; - time: time; - field: [string, ...string[]]; + aesthetic: string & keyof typeof dimensions; + time: 'current' | 'last'; + field: Some; }; const buffers: BufferSummary[] = []; - const priorities = [ + const priorities: (string & keyof typeof dimensions)[] = [ 'x', 'y', 'color', @@ -903,7 +902,7 @@ export class ReglRenderer extends Renderer { 'filter', 'filter2', 'foreground', - ] as (keyof typeof dimensions)[]; + ]; for (const aesthetic of priorities) { const times = ['current', 'last'] as const; for (const time of times) { @@ -937,7 +936,7 @@ export class ReglRenderer extends Renderer { return priorities.indexOf(a.aesthetic) - priorities.indexOf(b.aesthetic); }); - const aes_to_buffer_num: Record = {}; // eg 'x' => 3 + const aes_to_buffer_num: TupleMap = new TupleMap([]); // Pre-allocate the 'ix' buffer and the 'ix_in_tile' buffers. const variable_to_buffer_num: TupleMap = new TupleMap([ @@ -946,27 +945,36 @@ export class ReglRenderer extends Renderer { ]); // eg 'year' => 3 let num = 1; for (const { aesthetic, time, field } of buffers) { - const k = `${aesthetic}--${time}`; + const k: Some = [aesthetic, time]; + // console.log({ field }); if (variable_to_buffer_num.get(field) !== undefined) { - aes_to_buffer_num[k] = variable_to_buffer_num.get(field); + aes_to_buffer_num.set(k, variable_to_buffer_num.get(field)); continue; } if (num++ < 16) { - aes_to_buffer_num[k] = num; + aes_to_buffer_num.set(k, num); variable_to_buffer_num.set(field, num); continue; } else { // Don't use the last value, use the current value. // Loses animation but otherwise plots nicely. - // Strategy will break if more than 15 base channels are defined, + // Strategy will break if more than 14 base channels are defined, // which is not currently possible. - aes_to_buffer_num[k] = aes_to_buffer_num[`${aesthetic}--current`]; + aes_to_buffer_num.set( + [aesthetic, 'last'], + aes_to_buffer_num.get([aesthetic, 'current']), + ); } } - const buffer_num_to_variable = [...variable_to_buffer_num.keys()]; + // Store our maps on the object. this.aes_to_buffer_num = aes_to_buffer_num; this.variable_to_buffer_num = variable_to_buffer_num; + + // Build up a a list of which variable is at which buffer number. + const buffer_num_to_variable_vs = [...variable_to_buffer_num.entries()]; + buffer_num_to_variable_vs.sort((a, b) => a[1] - b[1]); + const buffer_num_to_variable = buffer_num_to_variable_vs.map((d) => d[0]); this.buffer_num_to_variable = buffer_num_to_variable; } @@ -1055,6 +1063,7 @@ export class BufferManager { if (tile.readyToUse) { // tile.get_column(keyset[0]); } else { + // pass } return false; } @@ -1200,7 +1209,7 @@ export function getNestedVector( col_names = col_names.filter((d) => !d.startsWith('_')); } throw new Error( - `Requested ${key} but table only has columns ["${col_names.join( + `Requested ${key.join('->')} but table only has columns ["${col_names.join( '", "', )}]"`, ); @@ -1213,11 +1222,13 @@ export function getNestedVector( column = column.getChild(k); } if (column.data.length !== 1) { - throw new Error(`Column ${key} has ${column.data.length} buffers, not 1.`); + throw new Error( + `Column ${key.join('->')} has ${column.data.length} buffers, not 1.`, + ); } if (!column.type || !column.type.typeId) { - throw new Error(`Column ${key} has no type.`); + throw new Error(`Column ${key.join('->')} has no type.`); } function assertNotStruct( value: Vector | Vector, @@ -1359,7 +1370,7 @@ export function neededFieldsToPlot( if (v && typeof v !== 'string' && v['field'] !== undefined) { const needed_key: Some = [v['field']]; if (v['subfield'] !== undefined) { - const subfield = v['subfield']; + const subfield: string | string[] = v['subfield'] as string | string[]; const asArray = Array.isArray(subfield) ? [...subfield] : [subfield]; needed_key.push(...asArray); } diff --git a/src/selection.ts b/src/selection.ts index 970086b38..b73a4821b 100644 --- a/src/selection.ts +++ b/src/selection.ts @@ -477,8 +477,6 @@ export class DataSelection { params.ids, params.idField, ).then(markReady); - // } else if (isBooleanColumnParam(params)) { - // void this.add_boolean_column(params.name, params.field).then(markReady); } else if (isFunctionSelectParam(params)) { void this.add_function_column(params.name, params.tileFunction).then( markReady, diff --git a/src/types.ts b/src/types.ts index a173d0811..6b9b629cc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -637,7 +637,7 @@ export type GlobalDrawProps = { use_scale_for_tiles: boolean; grid_mode: 1 | 0; buffer_num_to_variable: string[][]; - aes_to_buffer_num: Record; + aes_to_buffer_num: TupleMap; variable_to_buffer_num: TupleMap; color_picker_mode: 0 | 1 | 2 | 3; zoom_matrix: [ diff --git a/src/utilityFunctions.ts b/src/utilityFunctions.ts index 1cd76e6fa..9267ca47d 100644 --- a/src/utilityFunctions.ts +++ b/src/utilityFunctions.ts @@ -97,7 +97,7 @@ export type Some = [T, ...T[]]; /** * A Map that allows for tuples as keys with proper identity checks. */ -export class TupleMap { +export class TupleMap { private map: Map> = new Map(); private value?: V;