-
Notifications
You must be signed in to change notification settings - Fork 4
Simulation Experiment view: reset the runs upon changing the view's settings #380
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request addresses issue #378 by resetting tracked runs when simulation view settings change, while also implementing a significant performance optimization by converting data arrays from number[] to Float64Array throughout the codebase.
Changes:
- Implements settings comparison and automatic run reset when settings change in the Simulation Experiment view
- Converts C++ API bindings and TypeScript interfaces to use
Float64Arrayinstead ofnumber[]for better performance - Refactors settings dialog to pass model parameters as props rather than settings values
- Adds resize capability to GraphPanelWidget and removes some null checks
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/src/components/views/SimulationExperimentView.vue | Implements settings change detection and run reset logic; adds graph panel resize handling |
| src/renderer/src/libopencor/src/sed.cpp | Converts six functions to return Float64Array via doublesToNapiFloat64Array |
| src/renderer/src/libopencor/src/common.cpp | Refactors doublesToNapiArray to doublesToNapiFloat64Array using memcpy for performance |
| src/renderer/src/libopencor/src/common.h | Updates function signature to reflect Float64Array return type |
| src/renderer/src/libopencor/locSedApi.ts | Updates TypeScript interface return types from number[] to Float64Array |
| src/renderer/src/libopencor/locApi.ts | Updates API interface definitions for Float64Array return types |
| src/renderer/src/components/widgets/GraphPanelWidget.vue | Adds resize method, removes null checks, updates trace types to Float64Array |
| src/renderer/src/components/dialogs/SimulationExperimentViewSettingsDialog.vue | Moves model parameters from settings to props |
| src/renderer/src/common/locCommon.ts | Updates simulationData return type to Float64Array |
| src/renderer/src/components/ContentsComponent.vue | Minor formatting change to defineExpose |
| src/renderer/package.json | Version bump to 0.20260202.1 |
| package.json | Version bump to 0.20260202.1 |
Comments suppressed due to low confidence (1)
src/renderer/src/components/views/SimulationExperimentView.vue:693
- Type mismatch:
parserEvaluateis declared to returnFloat64Array, butparser.evaluate()from mathjs returnsmathjs.MathType, which may be a Matrix, Array, or other type depending on the operation. The.map()calls in the custom math functions (lines 591-616) will return regular arrays, not Float64Arrays. This will cause a type error when the returned values are used where Float64Arrays are expected.
const parserEvaluate = (value: string): Float64Array => {
// Note: we replace `*` and `/` (but not `.*` and `./`) with `.*` and `./`, respectively, to ensure element-wise
// operations.
return parser.evaluate(value.replace(/(?<!\.)\*(?!\.)/g, '.*').replace(/(?<!\.)\/(?!\.)/g, './'));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
It is indeed faster and more memory‑efficient for large simulation data and is the recommended data format for plotly.js.
… and `editableModelParameters` as props. This has nothing to do with the settings per se.
…hanges. This can happen when changing the settings and adding/removing a graph panel, in which case the graph panels won't resize. Unfortunately, this is most likely related to the way Plotly works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #378.