Filter lambda expressions failing to update filters correctly in some cases

Overall, behaviour is somewhat consistently wrong, with some blips, hinting at perhaps some sort of inconsistent state update or race condition.

Example:

The following updates are executed in order for this codebase https://github.com/davidnmora/lyric-viz which is deployed via GitHub pages here (reproduce easily using the categorical dropdown menu):

lambda: 'generic_genre => generic_genre === "HIP_HOP"' filters correctly
'generic_genre => generic_genre === "HIP_HOP" || generic_genre === "ROCK"' fails to trigger any update to the chart
'generic_genre => generic_genre === "HIP_HOP" || generic_genre === "ROCK" || generic_genre === "UNCATEGORIZED"' filters correctly

Note: I've observed this with both passing in functions as well as passing in strings to be converted to functions.

In other cases, toggling between filtering on 1 vs 2 categories triggers a visible re-painting of the scatter plot, but the plot is stuck on a 2-category filter state regardless. Below: red & blue categories are present, despite the current state being set to just show the blue category:

From @bmschmidt

The one thing scarce resource at play in both of these that might not be obvious are attribute buffers per point. There’s a hard limit of 16 dimensions we can send to the GPU about each point that has to include both current and past values for x, y, color, index, size, all filters, etc. My first guess is that the problem with filters has something to do with the efforts to juggle around some of these values and reuse buffers where possible.

If this is actually the issue, the code in question lives in the preivew below, if I'm understanding you right. After index & tile position, we have 14 spots left.

This bug observed seems to be that filter is only able to filter on 1 logical clause, adding multiple seems to fail to update. This seems to imply an issue with how the lambda filter function is being applied (at least naively).

But you're suggesting something more low-level as a starting place to investigate: that perhaps the bug has to do with something along the lines of, "We set filter a successfully by setting the attribute buffer correctly. Then the client updates to a new filter, filter b, however we improperly update the buffer, and so filter a remains in use, meaning we don't get a proper update".

I'll start digging a little deeper.

deepscatter/src/regl_rendering.ts

Lines 938 to 1025 in 825b605

    
           public allocate_aesthetic_buffers() { 
        
             // There are only 14 attribute buffers available to use, 
        
             // once we pass in the index and position in the tile. The order here determines 
        
             // how important it is to capture transitions for them; if 
        
             // 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 Encoding; 
        
               time: time; 
        
               field: string; 
        
             }; 
        
             const buffers: BufferSummary[] = []; 
        
             const priorities = [ 
        
               'x', 
        
               'y', 
        
               'color', 
        
               'x0', 
        
               'y0', 
        
               'size', 
        
               'jitter_radius', 
        
               'jitter_speed', 
        
               'filter', 
        
               'filter2', 
        
             ]; 
        
             for (const aesthetic of priorities) { 
        
               const times: time[] = ['current', 'last']; 
        
               for (const time of times) { 
        
                 try { 
        
                   if (this.aes.dim(aesthetic)[time].field) { 
        
                     buffers.push({ 
        
                       aesthetic, 
        
                       time, 
        
                       field: this.aes.dim(aesthetic)[time].field, 
        
                     }); 
        
                   } 
        
                 } catch (error) { 
        
                   this.reglframe.cancel(); 
        
                   this.reglframe = undefined; 
        
                   throw error; 
        
                 } 
        
               } 
        
             } 
        
             buffers.sort((a, b) => { 
        
               // Current values always come first. 
        
               if (a.time < b.time) { 
        
                 return -1; 
        
               } // current < last. 
        
               if (b.time < a.time) { 
        
                 return 1; 
        
               } 
        
               return priorities.indexOf(a.aesthetic) - priorities.indexOf(b.aesthetic); 
        
             }); 
        
             type encodingkey = keyof Encoding; 
        
             //todo not all encoding keys. 
        
             const aes_to_buffer_num: Record<encodingkey, number> = {}; // eg 'x' => 3 
        
             // Pre-allocate the 'ix' buffer. 
        
             const variable_to_buffer_num: Record<string, number> = { 
        
               ix: 0, 
        
               ix_in_tile: 1, 
        
             }; // eg 'year' =>  3 
        
             let num = 1; 
        
             for (const { aesthetic, time, field } of buffers) { 
        
               const k = `${aesthetic}--${time}`; 
        
               if (variable_to_buffer_num[field] !== undefined) { 
        
                 aes_to_buffer_num[k] = variable_to_buffer_num[field]; 
        
                 continue; 
        
               } 
        
               if (num++ < 16) { 
        
                 aes_to_buffer_num[k] = num; 
        
                 variable_to_buffer_num[field] = num; 
        
                 continue; 
        
               } else { 
        
                 // Don't use the last value, use the current value. 
        
                 // Strategy will break if more than 15 base channels are defined, 
        
                 // which is not currently possible. 
        
                 aes_to_buffer_num[k] = aes_to_buffer_num[`${aesthetic}--current`]; 
        
               } 
        
             } 
        
             const buffer_num_to_variable = [...Object.keys(variable_to_buffer_num)]; 
        
             this.aes_to_buffer_num = aes_to_buffer_num; 
        
             this.variable_to_buffer_num = variable_to_buffer_num; 
        
             this.buffer_num_to_variable = buffer_num_to_variable; 
        
           }

@davidnmora I'm seeing this myself too, starting to get frustrated by it. I suspect the core of the problem is in this area, where new prefs and passed and old values are cleared out.

Basically the situation is that a StatefulAesthetic is a bundle of two aesthetics, A and B; at render time one is used to calculate the previous filter state, and the other to calculate the current filter state. These aesthetics are reused to avoid costly GPU memory allocations, and every time a statefulaesthetic changes the current buffer is rotated to the 'previous' position and what used to be the 'previous' position is rotated to the current position and then updated with the current state. If a position does not receive an update, then we have to check to see if the last draw contained an update. If it did we update the buffers to the same state as each other (otherwise it would try interpolating from the old positions again.) If not, nothing happens.

That logic is a little complicated and has some failure cases.

https://github.com/nomic-ai/deepscatter/blob/main/src/AestheticSet.ts#L99-L114

There is a general update method here that determines which is the old and which the new state.

https://github.com/nomic-ai/deepscatter/blob/main/src/Aesthetic.ts#L386-L394

And then there's a little custom logic on the abstract filter here:

https://github.com/nomic-ai/deepscatter/blob/main/src/Aesthetic.ts#L689-L694

My guess is that the problem has to do with that custom logic, or something else related to determining which filter is active.. It's possible that just pulling out the custom logic on the filter class would fix the problem. The issue is that there needs to be some way to clear a filter, and I think I might have been a little lax about whether that is done by filter: {} or filter: null.
This is especially fraught because filter: undefined means something quite different. (That there wasn't an update at all, and that the past and previous buffers ought to now be in sync with each other.)

There should be a test here eventually, so just for my own sake I'm going to lay out a scenario for the state of aesthetics A and B in a larger StatefulAesthetic.

INITIALIZATION:
A--current--default state.
B--previous--default state.

REQUEST FILTER '< 10'
B--current--FILTER '< 10'
A--previous--default state
(swap positions, update current.)

UPDATE OTHER FIELDS, NO ACTION
A--current--FILTER '< 10'
B--previous--FILTER '<10'
(swap positions, update current)

UPDATE OTHER FIELDS, NO ACTION
A--current--FILTER '< 10'
B--previous--FILTER '<10'

(don't swap, state is good.)

SET FILTER TO {}
B--current--default state
A--previous--FILTER '<10'

SET FILTER TO HIDE ALL
A--current--HIDE ALL
B--previous--default state

Solved around 2.9--important to set filters to null, not {}.

	public allocate_aesthetic_buffers() {
	// There are only 14 attribute buffers available to use,
	// once we pass in the index and position in the tile. The order here determines
	// how important it is to capture transitions for them; if
	// 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 Encoding;
	time: time;
	field: string;
	};
	const buffers: BufferSummary[] = [];
	const priorities = [
	'x',
	'y',
	'color',
	'x0',
	'y0',
	'size',
	'jitter_radius',
	'jitter_speed',
	'filter',
	'filter2',
	];
	for (const aesthetic of priorities) {
	const times: time[] = ['current', 'last'];
	for (const time of times) {
	try {
	if (this.aes.dim(aesthetic)[time].field) {
	buffers.push({
	aesthetic,
	time,
	field: this.aes.dim(aesthetic)[time].field,
	});
	}
	} catch (error) {
	this.reglframe.cancel();
	this.reglframe = undefined;
	throw error;
	}
	}
	}

	buffers.sort((a, b) => {
	// Current values always come first.
	if (a.time < b.time) {
	return -1;
	} // current < last.
	if (b.time < a.time) {
	return 1;
	}
	return priorities.indexOf(a.aesthetic) - priorities.indexOf(b.aesthetic);
	});
	type encodingkey = keyof Encoding;
	//todo not all encoding keys.
	const aes_to_buffer_num: Record<encodingkey, number> = {}; // eg 'x' => 3

	// Pre-allocate the 'ix' buffer.
	const variable_to_buffer_num: Record<string, number> = {
	ix: 0,
	ix_in_tile: 1,
	}; // eg 'year' => 3
	let num = 1;
	for (const { aesthetic, time, field } of buffers) {
	const k = `${aesthetic}--${time}`;
	if (variable_to_buffer_num[field] !== undefined) {
	aes_to_buffer_num[k] = variable_to_buffer_num[field];
	continue;
	}
	if (num++ < 16) {
	aes_to_buffer_num[k] = num;
	variable_to_buffer_num[field] = num;
	continue;
	} else {
	// Don't use the last value, use the current value.
	// Strategy will break if more than 15 base channels are defined,
	// which is not currently possible.
	aes_to_buffer_num[k] = aes_to_buffer_num[`${aesthetic}--current`];
	}
	}

	const buffer_num_to_variable = [...Object.keys(variable_to_buffer_num)];
	this.aes_to_buffer_num = aes_to_buffer_num;
	this.variable_to_buffer_num = variable_to_buffer_num;
	this.buffer_num_to_variable = buffer_num_to_variable;
	}