-
Notifications
You must be signed in to change notification settings - Fork 24
feat: allow for overriding constants at runtime #767
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
Open
chrispcampbell
wants to merge
37
commits into
main
Choose a base branch
from
chris/470-override-constants-v2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add the ConstantDef interface and createConstantDef helper function to define constant value overrides at runtime. This is similar to LookupDef but simpler since constants are scalar values rather than data arrays. Also add PLAN.md with full implementation plan. Generated with Claude Code
Add constants field to RunModelOptions interface for passing constant overrides at runtime. Unlike lookups, constants do NOT persist across calls since initConstants resets them on every run. Add customConstants configuration option to ModelSpec and ResolvedModelSpec to control which constants can be overridden at runtime. Generated with Claude Code
Add setConstantImpl helper and setConstant function generation to the JS code generator. The setConstant function allows overriding constant values at runtime using a switch statement based on varIndex, similar to setLookup but simpler since constants are scalar values. Generated with Claude Code
Add setConstantImpl helper and setConstant function generation to the C code generator. The setConstant function allows overriding constant values at runtime using a switch statement based on varIndex, with a simpler signature than setLookup (no numPoints parameter). Generated with Claude Code
Add setConstant method to JsModel interface and integrate constant overrides into the runJsModel flow. Constants are applied after lookup overrides but before input overrides, allowing for flexible runtime configuration. Generated with Claude Code
Add wasmSetConstant native function wrapper and integrate constant overrides into the WasmModel.runModel flow. Constants are applied after lookup overrides but before inputs, reusing the lookup sub indices buffer for efficiency. Generated with Claude Code
Add getConstants() method to RunModelParams interface to retrieve constant overrides passed to the latest runModel call. This completes the parameter passing infrastructure for the override constants feature. Generated with Claude Code
Update OnRunModelFunc type to include constants in options and pass constants from RunModelParams to the onRunModel callback. This completes the parameter flow from user API through to model execution. Generated with Claude Code
Add comprehensive integration test for the override constants feature, testing: - Default constant values - Overriding constants by name and ID - Non-persistence of constant overrides across runs - 1D and 2D subscripted constants - Non-subscripted constants - Both synchronous and asynchronous model runners Generated with Claude Code
Add getConstants() method implementations to BufferedRunModelParams and ReferencedRunModelParams classes. For now, BufferedRunModelParams returns undefined (async/worker support can be added later). ReferencedRunModelParams properly stores and returns constants with varRef resolution. Generated with Claude Code
Add setConstant method to MockJsModel to fix interface implementation error. Remove unused ConstantDef import from wasm-model.ts. Generated with Claude Code
Implement encoding/decoding functions for constants to enable async/worker model runners to override constants at runtime. This includes: - Add getEncodedConstantBufferLengths(), encodeConstants(), and decodeConstants() functions in var-indices.ts - Update BufferedRunModelParams with constant buffer sections and encoding/decoding - Update PLAN.md with implementation details and progress tracking Constants are encoded with simpler structure than lookups (no offset/length metadata needed, just sequential values). The buffer format uses two sections: constantIndices (metadata) and constants (values). Generated with Claude Code
Update build-once.ts to properly include customConstants when creating the resolved model spec and spec.json file. This is needed for the compile package to generate the correct setConstant function body. - Add customConstants to specJson with default value of false - Add customConstants resolution logic similar to customLookups - Include customConstants in the returned ResolvedModelSpec Generated with Claude Code
Add customConstants field to ModelOptions interface and processor output. This ensures customConstants from config is passed through to the model spec. Note: Currently experiencing a TypeScript build error that needs investigation. The ModelSpec interface has customConstants defined, but TypeScript is not recognizing it during the plugin-config build. Generated with Claude Code
This reverts the change in 0fe10bf that caused local dependencies to be using published versions instead.
I added this to an issue comment for posterity
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #470
@ToddFincannonEI: See issue for details. There are a lot of changes here but they're all very similar (and simpler than) what was done for lookup overrides. I don't think any of it should have an impact on EPS since it's all opt-in. The only "breaking" change is that
runModelWithBuffershas two new parameters, which can be set to NULL if they are not needed.(Re: adding parameters to an existing C function. I don't like making a breaking change like this, but the alternatives for extending a C API, like adding an "options" struct parameter will be even more annoying. I don't expect we'll need to add any more buffer parameters after this change, so I don't want to spend time making this fancier right now.)
As noted in #766, I plan to merge that one first, and then I'll update this branch to align with those
runModelWithBufferschanges before I merge this one (after you review and approve both).AI disclosure: I used Claude Code (Opus 4.5) to implement the first round of changes according to my detailed specification (recorded in the issue). I reviewed and edited the code for consistency with existing code and conventions.