Skip to content

Commit

Permalink
clean transitions between struct-nested fields (#160)
Browse files Browse the repository at this point in the history 
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. 
<!-- ELLIPSIS_HIDDEN -->


----

> [!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<string, number>` with `TupleMap<string, number>` 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`.
> 
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=nomic-ai%2Fdeepscatter&utm_source=github&utm_medium=referral)<sup> for 5bcd6a8. It will automatically update as commits are pushed.</sup>


<!-- ELLIPSIS_HIDDEN -->
  • Loading branch information
@bmschmidt
bmschmidt authored Oct 16, 2024
1 parent 49841ca commit 1938625
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 80 deletions.
76 changes: 31 additions & 45 deletions dev/svelte/SwitchPositions.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((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',
// },
// },
// });
}
</script>

Expand Down
1 change: 0 additions & 1 deletion src/Deeptable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@ export class Deeptable {
if (dim === undefined) {
continue;
}
console.log({ dim });
dim = (dim as Field<Struct<any>>).type.children.find(
(d) => d.name === sub,
);
Expand Down
6 changes: 4 additions & 2 deletions src/aesthetics/AestheticSet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down Expand Up @@ -84,7 +84,7 @@ export class AestheticSet {
}
}

_neededFields: TupleSet<string> = new TupleSet();
_neededFields: TupleSet<string> = new TupleSet<string>();

get neededFields(): string[][] {
return [...this._neededFields.values()];
Expand Down Expand Up @@ -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) {
Expand Down
67 changes: 39 additions & 28 deletions src/regl_rendering.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export class ReglRenderer extends Renderer {
public reglframe?: REGL.Cancellable;
public bufferManager: BufferManager;

private aes_to_buffer_num?: Record<string, number>;
private aes_to_buffer_num: TupleMap<string, number> = new TupleMap();
private variable_to_buffer_num?: TupleMap<string, number>;
private buffer_num_to_variable?: string[][];

Expand Down Expand Up @@ -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: [
Expand All @@ -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() {
Expand Down Expand Up @@ -260,7 +261,6 @@ export class ReglRenderer extends Renderer {
a.tile_id
);
});
// console.log({ prop_list });
this._renderer(prop_list);
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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<string>;
};
const buffers: BufferSummary[] = [];
const priorities = [
const priorities: (string & keyof typeof dimensions)[] = [
'x',
'y',
'color',
Expand All @@ -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) {
Expand Down Expand Up @@ -937,7 +936,7 @@ export class ReglRenderer extends Renderer {
return priorities.indexOf(a.aesthetic) - priorities.indexOf(b.aesthetic);
});

const aes_to_buffer_num: Record<string, number> = {}; // eg 'x' => 3
const aes_to_buffer_num: TupleMap<string, number> = new TupleMap([]);

// Pre-allocate the 'ix' buffer and the 'ix_in_tile' buffers.
const variable_to_buffer_num: TupleMap<string, number> = new TupleMap([
Expand All @@ -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<string> = [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;
}

Expand Down Expand Up @@ -1055,6 +1063,7 @@ export class BufferManager {
if (tile.readyToUse) {
// tile.get_column(keyset[0]);
} else {
// pass
}
return false;
}
Expand Down Expand Up @@ -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(
'", "',
)}]"`,
);
Expand All @@ -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<DS.SupportedArrowTypes> | Vector<Struct>,
Expand Down Expand Up @@ -1359,7 +1370,7 @@ export function neededFieldsToPlot(
if (v && typeof v !== 'string' && v['field'] !== undefined) {
const needed_key: Some<string> = [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);
}
Expand Down
2 changes: 0 additions & 2 deletions src/selection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number>;
aes_to_buffer_num: TupleMap<string, number>;
variable_to_buffer_num: TupleMap<string, number>;
color_picker_mode: 0 | 1 | 2 | 3;
zoom_matrix: [
Expand Down
2 changes: 1 addition & 1 deletion src/utilityFunctions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export type Some<T> = [T, ...T[]];
/**
* A Map that allows for tuples as keys with proper identity checks.
*/
export class TupleMap<K = Object, V = Object> {
export class TupleMap<K = object, V = object> {
private map: Map<K, TupleMap<K, V>> = new Map();
private value?: V;

Expand Down

0 comments on commit 1938625

Please sign in to comment.