Skip to content

refactor: Extract common Data1D processing logic from sync modifiers #192

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.

Sign up for GitHub

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

refactor: Extract common Data1D processing logic from sync modifiers #192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87a68f9
refactor: Extract common Data1D processing logic from sync modifiers
cursoragent Nov 1, 2025
ffc014e
Address PR review comments: optimize processData1D and use destructuring
cursoragent Nov 1, 2025
544b2e6
Address additional PR review comments: optimize loop and use Object.a…
cursoragent Nov 1, 2025
94e1529
Merge branch 'main' into cursor/solve-issue-185-and-create-pr-c984
andeplane Nov 1, 2025
b33e5b2
Checkpoint before follow-up message
cursoragent Nov 1, 2025
f7e3a9c
Improve loop logic in processData1D helper function
cursoragent Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions src/modifiers/modifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,106 @@
import { ModifierInput, ModifierOutput } from "./types";
import { LMPModifier, Data1D } from "../types";

interface ModifierProps {
name: string;
active: boolean;
}

export interface ProcessedData1D {
data1D: Data1D | undefined;
hasData1D: boolean;
clearPerSync: boolean;
}

/**
* Shared helper function to process Data1D from LMPModifier.
* Extracts common logic for syncing computes, fixes, and variables.
*/
export function processData1D(
lmpModifier: LMPModifier,
data1D: Data1D | undefined,
input: ModifierInput,
everything: boolean,
syncDataPoints: boolean
): ProcessedData1D {
// Get data1DNamesWrapper and extract size, then delete immediately
const data1DNamesWrapper = lmpModifier.getData1DNames();
const data1DNamesSize = data1DNamesWrapper.size();
const hasData1D = data1DNamesSize > 0;
data1DNamesWrapper.delete(); // Delete WASM wrapper to prevent memory leak

if (data1DNamesSize === 0) {
return { data1D, hasData1D, clearPerSync: false };
}

const clearPerSync = lmpModifier.getClearPerSync();

if (data1D == null) {
data1D = {
data: [],
labels: [],
};
}

if (everything || syncDataPoints) {
// Data points is only for plotting figures
if (clearPerSync) {
// For histograms (compute rdf etc) we don't have time as x axis, so we clear every time
data1D.data = [];
}

const lengthBeforeWeStart = data1D.data.length; // Used to avoid copying all data every time

if (data1D.labels.length === 0) {
// First label is never visible
data1D.labels.push("x");
}

// Get data1DVector once before the loop for better performance
const data1DVector = lmpModifier.getData1D();

for (let j = 0; j < data1DNamesSize; j++) {
const lmpData = data1DVector.get(j);

if (data1D.labels.length - 1 === j) {
// Add missing labels
data1D.labels.push(lmpData.getLabel());
}

const numPoints = lmpData.getNumPoints();
const yValuesPointer = lmpData.getYValuesPointer() / 4;
const yValues = input.wasm.HEAPF32.subarray(
yValuesPointer,
yValuesPointer + numPoints,
);

if (j === 0) {
const xValuesPointer = lmpData.getXValuesPointer() / 4;
const xValues = input.wasm.HEAPF32.subarray(
xValuesPointer,
xValuesPointer + numPoints,
);
for (let k = lengthBeforeWeStart; k < numPoints; k++) {
data1D.data.push([xValues[k]]);
data1D.data[k].push(yValues[k]);
}
Comment on lines +77 to +86
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The xValues array is only used when j === 0. However, it is declared inside the loop, which means memory is allocated for it in each iteration, even when it's not used. This could lead to unnecessary memory allocations and potential performance overhead, especially if data1DNamesSize is large. Consider moving the declaration and assignment of xValues outside the loop, or only assign it when j === 0.

        const xValuesPointer = lmpData.getXValuesPointer() / 4;
        const xValues = input.wasm.HEAPF32.subarray(
          xValuesPointer,
          xValuesPointer + numPoints,
        );
        for (let k = lengthBeforeWeStart; k < numPoints; k++) {
          data1D.data.push([xValues[k]]);
          data1D.data[k].push(yValues[k]);
        }

} else {
for (let k = lengthBeforeWeStart; k < numPoints; k++) {
data1D.data[k].push(yValues[k]);
}
}

// Delete the Data1D copy to prevent memory leak
lmpData.delete();
}

// Delete the vector wrapper after the loop to prevent memory leak
data1DVector.delete();
}

return { data1D, hasData1D, clearPerSync };
}

class Modifier {
public name: string;
public key: string;
Expand Down
77 changes: 11 additions & 66 deletions src/modifiers/synccomputesmodifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Modifier from "./modifier";
import { ModifierInput, ModifierOutput } from "./types";
import { processData1D } from "./modifier";

interface SyncComputesModifierProps {
name: string;
Expand Down Expand Up @@ -52,72 +53,16 @@ class SyncComputesModifier extends Modifier {
compute.yLabel = compute.lmpCompute.getYLabel();
compute.scalarValue = compute.lmpCompute.getScalarValue();

// Get data1DNamesWrapper and extract size, then delete immediately
const data1DNamesWrapper = compute.lmpCompute.getData1DNames();
compute.hasData1D = data1DNamesWrapper.size() > 0;
const data1DNamesSize = data1DNamesWrapper.size();
data1DNamesWrapper.delete(); // Delete WASM wrapper to prevent memory leak

if (data1DNamesSize > 0) {
compute.clearPerSync = compute.lmpCompute.getClearPerSync();

if (compute.data1D == null) {
compute.data1D = {
data: [],
labels: [],
};
}

if ((everything || compute.syncDataPoints) && compute.data1D) {
// Data points is only for plotting figures
if (compute.clearPerSync) {
// For histograms (compute rdf etc) we don't have time as x axis, so we clear every time
compute.data1D.data = [];
}

const lengthBeforeWeStart = compute.data1D.data.length; // Used to avoid coping all data every time

if (compute.data1D.labels.length === 0) {
// First label is never visible
compute.data1D.labels.push("x");
}

// Get data1DVector once before the loop for better performance
const data1DVector = compute.lmpCompute.getData1D();

for (let j = 0; j < data1DNamesSize; j++) {
const lmpData = data1DVector.get(j);

if (compute.data1D.labels.length - 1 === j) {
// Add missing labels
compute.data1D.labels.push(lmpData.getLabel());
}

const xValuesPointer = lmpData.getXValuesPointer() / 4;
const yValuesPointer = lmpData.getYValuesPointer() / 4;
const xValues = input.wasm.HEAPF32.subarray(
xValuesPointer,
xValuesPointer + lmpData.getNumPoints(),
) as Float32Array;
const yValues = input.wasm.HEAPF32.subarray(
yValuesPointer,
yValuesPointer + lmpData.getNumPoints(),
) as Float32Array;
for (let k = lengthBeforeWeStart; k < xValues.length; k++) {
if (j === 0) {
compute.data1D.data.push([xValues[k]]);
}
compute.data1D.data[k].push(yValues[k]);
}

// Delete the Data1D copy to prevent memory leak
lmpData.delete();
}

// Delete the vector wrapper after the loop to prevent memory leak
data1DVector.delete();
}
}
const processed = processData1D(
compute.lmpCompute,
compute.data1D,
input,
everything,
compute.syncDataPoints,
);
compute.data1D = processed.data1D;
compute.hasData1D = processed.hasData1D;
compute.clearPerSync = processed.clearPerSync;
}
output.computes[name] = compute;
}
Expand Down
76 changes: 11 additions & 65 deletions src/modifiers/syncfixesmodifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Modifier from "./modifier";
import { ModifierInput, ModifierOutput } from "./types";
import { processData1D } from "./modifier";

interface SyncFixesModifierProps {
name: string;
Expand Down Expand Up @@ -49,71 +50,16 @@ class SyncFixesModifier extends Modifier {
fix.yLabel = fix.lmpFix.getYLabel();
fix.scalarValue = fix.lmpFix.getScalarValue();

// Get data1DNamesWrapper and extract size, then delete immediately
const data1DNamesWrapper = fix.lmpFix.getData1DNames();
fix.hasData1D = data1DNamesWrapper.size() > 0;
const data1DNamesSize = data1DNamesWrapper.size();
data1DNamesWrapper.delete(); // Delete WASM wrapper to prevent memory leak

if (data1DNamesSize > 0) {
fix.clearPerSync = fix.lmpFix.getClearPerSync();
if (fix.data1D == null) {
fix.data1D = {
data: [],
labels: [],
};
}

if ((everything || fix.syncDataPoints) && fix.data1D) {
// Data points is only for plotting figures
if (fix.clearPerSync) {
// For histograms (compute rdf etc) we don't have time as x axis, so we clear every time
fix.data1D.data = [];
}

const lengthBeforeWeStart = fix.data1D.data.length; // Used to avoid coping all data every time

if (fix.data1D.labels.length === 0) {
// First label is never visible
fix.data1D.labels.push("x");
}

// Get data1DVector once before the loop for better performance
const data1DVector = fix.lmpFix.getData1D();

for (let j = 0; j < data1DNamesSize; j++) {
const lmpData = data1DVector.get(j);

if (fix.data1D.labels.length - 1 === j) {
// Add missing labels
fix.data1D.labels.push(lmpData.getLabel());
}

const xValuesPointer = lmpData.getXValuesPointer() / 4;
const yValuesPointer = lmpData.getYValuesPointer() / 4;
const xValues = input.wasm.HEAPF32.subarray(
xValuesPointer,
xValuesPointer + lmpData.getNumPoints(),
) as Float32Array;
const yValues = input.wasm.HEAPF32.subarray(
yValuesPointer,
yValuesPointer + lmpData.getNumPoints(),
) as Float32Array;
for (let k = lengthBeforeWeStart; k < xValues.length; k++) {
if (j === 0) {
fix.data1D.data.push([xValues[k]]);
}
fix.data1D.data[k].push(yValues[k]);
}

// Delete the Data1D copy to prevent memory leak
lmpData.delete();
}

// Delete the vector wrapper after the loop to prevent memory leak
data1DVector.delete();
}
}
const processed = processData1D(
fix.lmpFix,
fix.data1D,
input,
everything,
fix.syncDataPoints,
);
fix.data1D = processed.data1D;
fix.hasData1D = processed.hasData1D;
fix.clearPerSync = processed.clearPerSync;
output.fixes[name] = fix;
}
};
Expand Down
76 changes: 11 additions & 65 deletions src/modifiers/syncvariablesmodifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Modifier from "./modifier";
import { ModifierInput, ModifierOutput } from "./types";
import { processData1D } from "./modifier";

interface SyncVariablesModifierProps {
name: string;
Expand Down Expand Up @@ -51,71 +52,16 @@ class SyncVariablesModifier extends Modifier {
variable.scalarValue = variable.lmpVariable.getScalarValue();
variable.hasScalarData = variable.lmpVariable.hasScalarData();

// Get data1DNamesWrapper and extract size, then delete immediately
const data1DNamesWrapper = variable.lmpVariable.getData1DNames();
variable.hasData1D = data1DNamesWrapper.size() > 0;
const data1DNamesSize = data1DNamesWrapper.size();
data1DNamesWrapper.delete(); // Delete WASM wrapper to prevent memory leak

if (data1DNamesSize > 0) {
variable.clearPerSync = variable.lmpVariable.getClearPerSync();
if (variable.data1D == null) {
variable.data1D = {
data: [],
labels: [],
};
}

if ((everything || variable.syncDataPoints) && variable.data1D) {
// Data points is only for plotting figures
if (variable.clearPerSync) {
// For histograms (compute rdf etc) we don't have time as x axis, so we clear every time
variable.data1D.data = [];
}

const lengthBeforeWeStart = variable.data1D.data.length; // Used to avoid coping all data every time

if (variable.data1D.labels.length === 0) {
// First label is never visible
variable.data1D.labels.push("x");
}

// Get data1DVector once before the loop for better performance
const data1DVector = variable.lmpVariable.getData1D();

for (let j = 0; j < data1DNamesSize; j++) {
const lmpData = data1DVector.get(j);

if (variable.data1D.labels.length - 1 === j) {
// Add missing labels
variable.data1D.labels.push(lmpData.getLabel());
}

const xValuesPointer = lmpData.getXValuesPointer() / 4;
const yValuesPointer = lmpData.getYValuesPointer() / 4;
const xValues = input.wasm.HEAPF32.subarray(
xValuesPointer,
xValuesPointer + lmpData.getNumPoints(),
) as Float32Array;
const yValues = input.wasm.HEAPF32.subarray(
yValuesPointer,
yValuesPointer + lmpData.getNumPoints(),
) as Float32Array;
for (let k = lengthBeforeWeStart; k < xValues.length; k++) {
if (j === 0) {
variable.data1D.data.push([xValues[k]]);
}
variable.data1D.data[k].push(yValues[k]);
}

// Delete the Data1D copy to prevent memory leak
lmpData.delete();
}

// Delete the vector wrapper after the loop to prevent memory leak
data1DVector.delete();
}
}
const processed = processData1D(
variable.lmpVariable,
variable.data1D,
input,
everything,
variable.syncDataPoints,
);
variable.data1D = processed.data1D;
variable.hasData1D = processed.hasData1D;
variable.clearPerSync = processed.clearPerSync;
output.variables[name] = variable;
}
};
Expand Down