Draft: Add SciMLLogging support to MTK - #5014
Draft
jClugstor wants to merge 5 commits into
Draft
Conversation
Introduces `MTKVerbosity`, a SciMLLogging.jl v2 verbosity specifier for
ModelingToolkit diagnostics, defined via `@verbosity_specifier` in
ModelingToolkitBase. It has 16 toggles in four groups (compilation,
initialization, problem_construction, analysis), the five standard
presets (Standard preserves current default behavior exactly), and an
`initialization_verbosity` sub-specifier that carries the verbosity of
the initialization problem's solve as a preset or a solver specifier.
Also adds the private helpers the call sites build on:
- `_process_verbose_param`: normalize `MTKVerbosity`/preset/`Bool`
- `_route_problem_verbose`: type-routing for the problem-constructor
`verbose` keyword shared with the solver
- `_toggle_enabled`: guard for work that only serves a message
- `_override_toggle`: apply deprecated boolean keywords as overrides,
rebuilding the immutable specifier (and flipping `{Enabled}` when an
explicit `true` must win over `MTKVerbosity(None())`)
`MTKVerbosity` is exported from ModelingToolkitBase (and reexported by
ModelingToolkit) and pinned in the qa REEXPORTED_API list. SciMLLogging
is already in the transitive dependency tree via DiffEqBase/SciMLBase,
so this adds no new indirect dependencies.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`mtkcompile` gains a `verbose = Standard()` keyword (an MTKVerbosity, a SciMLLogging preset, or a Bool), normalized once at the public entry and forwarded explicitly by name through the whole pipeline: `_mtkcompile`, both `__mtkcompile` methods, `mtkcompile!`, `_mtkcompile!`, `_mtkcompile_worker!`, `alias_elimination!`, `tearing`, and `dummy_derivative`. The last three consume it so it never leaks into StateSelection/ModelingToolkitTearing kwarg sinks; handing it to those packages when they adopt SciMLLogging is a one-line change each. The four compile-path logging sites become `@SciMLMessage` toggles: - `state_priority_tie` (alias elimination tie warning) - `underconstrained_variables` (replaces the undocumented `print_underconstrained_variables` kwarg, now emitted only when the set is non-empty) - `if_lifting_condition_grammar` (CondRewriter carries the specifier; passes in `additional_passes` may opt in by accepting `(sys, verbose::MTKVerbosity)`, which is how IfLifting receives it) - `observed_equation_cycle` (replaces the TOPSORT_EQS_PRINT_CYCLE global, kept as a deprecated escape hatch; the SCC diagnostic is now computed lazily inside the message closure) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`mtkcompile!` has no local `simplify` binding, so the bare `simplify` in this `_mtkcompile!` call passed the function `Symbolics.simplify` as the value of a `simplify` keyword, which then rode the `kwargs...` sink harmlessly but incorrectly. Pre-existing bug, unrelated to the verbosity work beyond being adjacent to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Converts the remaining easily-reachable @warn/@info sites (18 total) to MTKVerbosity toggles and routes the problem-constructor `verbose` keyword, which problem kwargs share with the solver: - `SciMLProblemOptions` replaces its `warn_initialize_determined` and `warn_cyclic_dependency` Bool fields with one `verbosity` field. The keyword constructor is the single routing point: an MTKVerbosity is consumed by MTK and removed from `prob.kwargs` (in `filter_kwargs`); presets and Bools apply to MTK and are still forwarded to `solve` as before; solver specifiers (e.g. DEVerbosity) pass through untouched. - The deprecated boolean keywords remain accepted everywhere they existed and override the corresponding toggles when explicitly passed (`warn_initialize_determined` maps to the singular/over/ underdetermined initialization toggles, `warn_cyclic_dependency` to `cyclic_dependency`, `warn_empty_op` to `empty_operating_point`). - The `initialization_verbosity` sub-specifier is forwarded as the initialization problem's solve-time `verbose` (default `Minimal()`), replacing the hardcode from the initialization_verbosity branch. Note: an outer `verbose = true`/DEVerbosity no longer leaks verbatim into `initializeprob.kwargs`; the sub-specifier value goes there. - Sites converted: cyclic-dependency reports (x2, now guarded so the cycle search only runs when the toggle is enabled), singular/over/ underdetermined initialization (MTKBase fallback and MTK override), SCC-requires-split, BVProblem/dynamic-opt overdetermination, missing SCC schedule, dynamic-opt dt/steps warnings, linearization_function's empty operating point, analyze_initialization_jacobian's report, generate_control_function's unbound-inputs warning, and analysis-point causality. Incidental fixes: `process_DynamicOptProblem` warned about overdetermination twice per problem (duplicate call removed); `linearization_function`'s `warn_initialize_determined` kwarg was captured but never forwarded, so it never did anything (it now does); the analysis-point message advertised a nonexistent `warn = false` keyword (the kwarg is `verbose`). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New test files in both packages (wired into the InterfaceI groups): specifier construction (presets, groups, sub-specifier, kwarg precedence), `_process_verbose_param`/`_override_toggle` semantics (including an explicit deprecated Bool beating `MTKVerbosity(None())`), per-toggle emit/silence for every locally triggerable site, problem-constructor type routing asserted via `prob.kwargs`, and `initialization_verbosity` forwarding asserted via `initializeprob.kwargs`. Debugging.md gains a "Controlling verbosity" section with the full toggle table, the problem-constructor routing rules, and the deprecated-keyword equivalences; the `show_api_guidance!` note now points at MTKVerbosity as the arriving replacement direction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.