feat: parameters shared by several graphs, from parameter files - #21
Merged
Merged
Conversation
A stage argument can name a parameter, `Truncate(width=$text.width)`, whose value comes from a JSON parameter file that several graph files share. A graph file names its parameter files with `params "file.json"` lines, relative to the graph file; later files override earlier ones member by member (JSON merge patch). - dsl::loadGraphProgram(path, overrides) reads a graph file with its parameter files and binds them; parseGraphProgram(text, parameters), bindParameters and loadParameters bind values from the caller. - Dots walk into nested objects, and a parameter can hold any JSON value, so lists and objects can now reach a stage from the DSL. - An unknown name is a located diagnostic with a suggestion; an unreadable parameter file is one at its params line; unused parameters are not reported. Unbound parameters are reported as not set when a graph is built, and renderings show them as `$name`. - validateDslGraph also takes a parsed GraphProgram. - New sample apps/tunedPipeline and EXAMPLE.md section 11.
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
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.
Why
Several pipelines often need the same tuning values, such as a width or a threshold. Today each
.fggraph repeats them inline, so keeping them in sync depends on remembering to edit every copy. This PR moves them into a parameter file that graphs name and share.What
$namearguments. Dots walk into nested objects. A parameter can hold any JSON value, so lists and objects can now reach a stage from the DSL, which partly lifts the "flat arguments" limitation.params "file.json"lines. Paths are relative to the graph file. Files apply in order, each overriding the ones before it member by member (JSON merge patch). Theoverridesargument ofloadGraphProgramapplies last.GraphParameters.hpp, included byDslFilterGraph.hpp:dsl::loadGraphProgram(path, overrides)dsl::parseGraphProgram(text, parameters)dsl::bindParameters(program, parameters)dsl::loadParameters(path)validateDslGraphnow also takes a parsedGraphProgram.1:22: unknown parameter '$text.widht' — did you mean '$text.width'?paramsline, with no follow-on "unknown parameter" noise.parseGraphProgram(text)only records references and files. Renderings (toAscii/toMermaid/toDot) show unbound parameters as$name. Building a graph from an unbound program reports each parameter as "not set".paramskeeps working, because a directive needs a string literal right after the keyword. The deprecated hand-written parser does not support the new syntax (noted in its header and the CHANGELOG).Docs and sample
apps/tunedPipeline: two.fgfiles,tuning.json,report.json.Testing
tests/FilterGraphTests/ParameterTests.cpp: 16 cases covering binding, nested names, mixing with literals, diagnostics and their locations, syntax errors, rendering, file loading, relative paths, override order and unreadable files.Out of scope (possible follow-ups)
Stage($preset).