From 8bfa71d18df615657f0f510883fb8d4d8b3d5298 Mon Sep 17 00:00:00 2001 From: odunbar Date: Wed, 20 May 2026 20:37:27 -0700 Subject: [PATCH 1/5] add claude settings and skills --- .claude/settings.json | 69 ++ .claude/skills/base-show/SKILL.md | 533 ++++++++++++ .claude/skills/docstrings/SKILL.md | 548 ++++++++++++ .claude/skills/error-message-manager/SKILL.md | 818 ++++++++++++++++++ 4 files changed, 1968 insertions(+) create mode 100644 .claude/settings.json create mode 100644 .claude/skills/base-show/SKILL.md create mode 100644 .claude/skills/docstrings/SKILL.md create mode 100644 .claude/skills/error-message-manager/SKILL.md diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..4990e57 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,69 @@ +{ + "$schema": "https://json.schemastore.org/claude-code-settings.json", + "permissions": { + "allow": [ + "Bash(julia --project *)", + "Read(*)", + "FileWrite(src/*, test/*, docs/*, examples/*, ai/*)", + "Edit(src/*)", + "Write(src/*)", + "Bash(git log*)", + "Bash(git diff*)", + "Bash(git status*)", + "Bash(git show*)", + "Bash(git blame*)", + "Bash(git ls-files*)", + "Bash(git rev-parse*)", + "Bash(git describe*)", + "Bash(git shortlog*)", + "Bash(git tag -l*)", + "Bash(git tag --list*)", + "Bash(git stash list*)", + "Bash(git stash show*)", + "Bash(git remote -v*)", + "Bash(git remote show*)", + "Bash(git branch -l*)", + "Bash(git branch --list*)", + "Bash(git branch -a*)", + "Bash(git branch -r*)" + ], + "deny": [ + "Read(./.env)", + "Bash(git commit*)", + "Bash(git add*)", + "Bash(git push*)", + "Bash(git pull*)", + "Bash(git merge*)", + "Bash(git rebase*)", + "Bash(git checkout*)", + "Bash(git switch*)", + "Bash(git restore*)", + "Bash(git reset*)", + "Bash(git cherry-pick*)", + "Bash(git clean*)", + "Bash(git rm*)" + ] + }, + "hooks": { + "UserPromptSubmit": [ + { + "hooks": [ + { + "type": "command", + "command": "printf '\\n---\\nSKILL ROUTING: Review the available skills listed in the system context. If one clearly matches this task, invoke it via the Skill tool before proceeding. If no skill clearly matches, use AskUserQuestion to ask the user: (a) whether they would like to apply any of the listed skills to this task anyway, (b) whether they would like to create a new skill using the skill-creator skill — in which case propose a suggested skill name and one-line description tailored to this task — or (c) proceed without a skill. Also use AskUserQuestion to ask the user whether they would like to use subagents (e.g., Explore, Plan, general-purpose) for this task, and if so, how (e.g., for research, parallel work, or isolated execution). Do NOT proceed with the task until the user has answered.\\n'" + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "julia --project=.dev .dev/climaformat.jl . 2>/dev/null || echo 'Formatting skipped as error encountered when calling `julia --project=.dev .dev/climaformat.jl`'" + } + ] + } + ] + } +} diff --git a/.claude/skills/base-show/SKILL.md b/.claude/skills/base-show/SKILL.md new file mode 100644 index 0000000..9bf6775 --- /dev/null +++ b/.claude/skills/base-show/SKILL.md @@ -0,0 +1,533 @@ +--- +name: base-show +description: > + Add concise Base.show and Base.summary methods to Julia types whose default REPL + representation is unhelpful or overwhelming. Use this skill whenever the user + mentions that a type prints badly in the REPL, asks to improve how an object is + displayed or printed, wants a custom show, summary, or repr for a Julia type, or + says the REPL output is noisy, verbose, or hard to read. Also trigger when the user + asks to "make the REPL output nicer", "add a show method", "add a summary method", + "customize display", or "fix what prints when I type a variable name". This skill + produces compact, informative Base.show and Base.summary methods and matching unit + tests — invoke it proactively whenever show, summary, display, print, repr, + or REPL output is mentioned in a Julia context. Good candidates in RandomFeatures.jl + include Decomposition, ScalarFeature, VectorFeature, RandomFeatureMethod, and Fit — + all of which hold matrices or nested objects that print poorly by default. +--- + +# base-show + +Add concise `Base.show(io::IO, ::MIME"text/plain", x::T)` and `Base.summary(io::IO, +x::T)` methods to Julia types whose default REPL representation is unhelpful or +overwhelming. Julia's default show dumps every field recursively; types that hold +matrix decompositions, large parameter distributions, nested arrays, or many scalar +fields produce screens of unreadable text at the REPL. + +`Base.show(io, MIME"text/plain", x)` must also handle the `:compact` IOContext key. +When Julia renders an object as an element inside a container (e.g. printing a +`Vector{MyType}`), it sets `:compact => true` on `io`. Without a compact branch the +full multi-line output is repeated for every element, producing an unreadable wall of +text. The compact branch must produce exactly one line (no newlines), giving the same +kind of at-a-glance hint as `Base.summary`. + +This skill produces both methods and accompanying unit tests so that interactive use +of the package is pleasant without losing key summary information. + +## Workflow + +### Step 0 — Audit existing show methods (retrofit mode) + +Skip this step if you are adding show methods to types that have none. Apply it when +the user asks to retrofit existing show methods — e.g. to add the compact branch to +methods that were written before this protocol existed. + +**Find MIME methods that lack the compact branch:** + +``` +grep -n 'MIME"text/plain"' src/show.jl +``` + +For each match, check whether the function body contains `get(io, :compact`. Any that +do not are candidates for retrofit. + +**Detect the old forwarding anti-pattern (infinite-recursion risk):** + +``` +grep -nA2 'function Base\.show(io::IO, x::' src/ | grep 'show(io, MIME' +``` + +If this matches, a 2-arg `show(io, x)` is calling the MIME method — the *wrong* +direction. Once the MIME method gains a compact branch that calls `show(io, x)`, you +get infinite recursion. Flag every match and reverse the direction: the 2-arg method +becomes the compact one-liner, and the MIME method calls it via `show(io, x)` in its +compact branch. + +**Identify pre-existing bespoke 2-arg shows:** + +A bespoke 2-arg show is one that already exists but does not follow summary style — +for example, it may omit the type name entirely or use a different format. Check each +existing `Base.show(io::IO, x::T)` against its paired `Base.summary`. If the outputs +differ substantially, the 2-arg show is bespoke and needs a custom compact test (see +Step 4). + +### Step 1 — Enumerate concrete types + +List every concrete (non-abstract) struct defined in the package source: + +``` +grep -nrE '^(mutable )?struct ' src/ +``` + +Exclude `abstract type` declarations — they cannot be instantiated and do not need +show methods. + +### Step 2 — Classify show noisiness + +For each concrete type, decide whether its default show output would be noisy. A type +is noisy if it holds at least one of: + +- A matrix decomposition or large `AbstractMatrix` +- A `ParameterDistribution` or similar composite distribution object +- A `Dict` with potentially many entries +- A large or variable-length `Array` +- Another struct that is itself noisy +- More than approximately six fields in total + +Also run: + +``` +grep -nrE 'Base\.(show|summary)' src/ +``` + +Skip any type that already has a custom `Base.show` or `Base.summary` method — do not +overwrite existing customization. + +### Step 3 — Write show and summary methods + +For each noisy type without existing methods, write **both** a `Base.show` and a +`Base.summary` method. + +**`Base.show`** — always write two overloads together: + +```julia +# 3-arg MIME method: full REPL display, with compact fallback +function Base.show(io::IO, ::MIME"text/plain", x::T) + if get(io, :compact, false) + show(io, x) # delegate to the 2-arg compact method + else + println(io, "T") + println(io, " field_name : ", summary_value) + # ... + end +end + +# 2-arg method: single-line compact representation (no newline) +function Base.show(io::IO, x::T) + print(io, "T (key_hint)") +end +``` + +The 3-arg (MIME) non-compact branch must: + +- Print the type name (and any cheap size hints) on the first line. +- Follow with 1–5 concise summary lines: counts, sizes, or ranges of important fields. + Never print collection contents. +- Produce at most 10 lines of output for any valid instance, including edge cases such + as empty collections or zero-element structs. + +The 2-arg method (compact representation) must: + +- Produce exactly one line with no trailing newline. +- Match `Base.summary` style: type name followed by the most essential identifying hint + in parentheses — e.g. `"ScalarFeature (256 features, Cosine)"`. +- Remain O(1): no loops, no collection materialisation. + +Julia calls the 2-arg method when rendering elements inside containers (arrays, dicts, +etc.), passing `io` with `:compact => true`. The MIME method's compact branch delegates +to it so both paths produce the same single-line output. + +**`Base.summary`** — single-line description used when the object appears inside a +container or is printed in a broader context (e.g., as an element of a `Vector`): + +```julia +function Base.summary(io::IO, x::T) + print(io, "T (key_hint)") +end +``` + +The method must: + +- Fit on one line — no newlines. +- Convey the most important size or identity hint (e.g., number of features, output + dimension, matrix size), so the reader immediately knows what they are looking at. +- Remain cheap: O(1) field accesses only. + +Good examples of what to put in the hint: `"256 features"`, `"10×10 SVD"`, +`"empty"`. Avoid repeating the type name verbatim as the only content — add value. + +**Placement**: place both methods adjacent to their type definition in the same source +file, or gather all show/summary methods in a dedicated `src/show.jl` included from +the main module file. Follow whatever convention is already present in the package; +default to `src/show.jl` if no prior convention exists. + +If creating `src/show.jl`, add `include("show.jl")` to the main module file after the +type definitions it references. + +### Step 4 — Write unit tests + +Write one test block per type, covering `show` (full and compact), and `summary`. Each +test block must: + +- Construct a minimal valid instance of the type. +- For full show: capture output with `sprint(show, MIME("text/plain"), instance)` and + assert that it contains the type name and that line count does not exceed 10. +- For compact show: capture `out2 = sprint(show, instance)` (2-arg) and assert it + contains the type name and has no `'\n'`. Also capture + `out3 = sprint(show, MIME("text/plain"), instance; context=:compact => true)` and + assert `out2 == out3` — both compact paths must agree. +- For `summary`: capture output with `sprint(summary, instance)` and assert that it + contains the type name and produces exactly one line (no `'\n'` in output). + +**Bespoke 2-arg shows (retrofit case):** Some types may already have a 2-arg show +that intentionally does not include the type name or follow summary style — the method +is doing something custom. Using a shared `check_compact(x, typename)` helper will +fail the typename assertion for these. Instead, write a hand-rolled compact test: + +```julia +s2 = sprint(show, instance) +@test !occursin('\n', s2) # no newline +@test s2 == sprint(show, MIME("text/plain"), instance; context = :compact => true) # paths agree +``` + +Avoid asserting exact strings so that cosmetic changes to the output do not break tests. + +### Step 5 — Verify + +Run the package test suite: + +``` +julia --project -e 'using Pkg; Pkg.test()' +``` + +Confirm that all new tests pass and no pre-existing tests regress. + +### Step 6 — Offer to improve the skill + +After the tests pass and the REPL output looks good, ask the user: "Would you like to improve the **base-show** skill itself using skill-creator? You can suggest changes to the workflow or quality criteria, or I can analyse what came up during this session to identify improvements to the skill." + +## Common patterns + +### Two-overload pattern (always write both together) + +Always define the 2-arg and 3-arg MIME overloads as a pair. The MIME method's compact +branch calls the 2-arg method, so both display paths (REPL and in-container) converge +on the same one-liner without repetition: + +```julia +function Base.show(io::IO, ::MIME"text/plain", x::RandomFeatureMethod) + if get(io, :compact, false) + show(io, x) + else + println(io, "RandomFeatureMethod") + # ... full multi-line body ... + end +end + +function Base.show(io::IO, x::RandomFeatureMethod) + print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), + ", n=", get_n_features(x.random_feature), ")") +end +``` + +Without the 2-arg method, `[rfm]` in a `Vector` falls back to Julia's default field +dump. Without the compact branch in the MIME method, the same dump appears whenever +the object is embedded in a container that happens to call `show(io, MIME"text/plain", +x)` with `:compact => true`. + +### Truncate long collections with "… and N more" + +When a type holds a variable-length collection, cap the loop to keep output bounded: + +```julia +function Base.show(io::IO, ::MIME"text/plain", x::Fit) + n = length(x.coeffs) + println(io, "Fit with ", n, " coefficient", n == 1 ? "" : "s") + max_show = 8 + for i in 1:min(n, max_show) + println(io, " [", i, "]: ", round(x.coeffs[i], sigdigits = 4)) + end + n > max_show && println(io, " … and ", n - max_show, " more") +end +``` + +### Conditional fields + +Only print a field when it carries information: + +```julia +if !isnothing(x.feature_parameters) + println(io, " feature_params : ", sprint(summary, x.feature_parameters)) +end +``` + +### Pluralisation in summary + +Match English grammar for counts that can be 0 or 1: + +```julia +print(io, "ScalarFeature (", n, " feature", n == 1 ? "" : "s", ", ", sf_name, ")") +``` + +### Arrow notation for mappings + +Use `→` in summary when the type represents a transformation between spaces: + +```julia +print(io, "VectorFeature (", n_features, " features → ", output_dim, "D output)") +``` + +### Unicode in mathematical contexts + +Use `×` for matrix dimensions, `→` for transformations, and `|u|` for set sizes. +These are rendered cleanly in all modern Julia terminals and communicate mathematical +meaning concisely. + +```julia +# Decomposition summary: Decomposition (10×10, SVD) +m, n = size(x.full_matrix) +print(io, "Decomposition (", m, "×", n, ", ", nameof(typeof(x.decomposition)), ")") +``` + +### Use nameof for parametric type identity + +When a type carries a type-parameter that identifies its variant, use `nameof` rather +than printing the full parameterised name: + +```julia +# ScalarFeature{Cosine} → print "Cosine", not the full type string +print(io, "ScalarFeature (", x.n_features, " features, ", + nameof(typeof(x.scalar_function)), ")") +``` + +### Section separators in show.jl + +When collecting all methods in a dedicated `show.jl`, organise by type family with +aligned comment rulers: + +```julia +# ── ScalarFeatures ──────────────────────────────────────────────────────────── +# ── VectorFeatures ──────────────────────────────────────────────────────────── +# ── RandomFeatureMethod / Fit ───────────────────────────────────────────────── +# ── Utilities / Decomposition ───────────────────────────────────────────────── +``` + +## Quality criteria + +| Criterion | Priority | Definition | +|---|---|---| +| Coverage | High | Every type classified as noisy in Step 2 has a `Base.show` (both overloads) and a `Base.summary` method. | +| Compact support | High | The 3-arg MIME `show` checks `get(io, :compact, false)` and calls the 2-arg `show(io, x)` in the compact branch. The 2-arg method produces exactly one line with no newline. | +| Brevity — show | High | Full (non-compact) show output is at most 10 lines for any valid instance, including edge cases. | +| Brevity — summary | High | Summary output is exactly one line (no newlines) for any valid instance. | +| Safety | High | Neither method throws on any valid instance. | +| Allocation-safety | High | All data access is O(1): use `length()`, `size()`, `isempty()`, or `first()` on lazy iterators. Never call `collect()`, `sort()`, `filter()`, or any function that materialises a new collection. | +| Test robustness | Medium | Tests assert structural properties, not exact strings. Cosmetic changes do not break tests. | +| No regression | High | Pre-existing tests continue to pass; no unintended changes to other source files. | + +## Formatting rules + +- **MIME show signature**: `Base.show(io::IO, ::MIME"text/plain", x::MyType)` +- **MIME show structure**: always starts with `if get(io, :compact, false); show(io, x); else ... end`. +- **MIME show full branch — first line**: type name via `println(io, "TypeName")`. Cheap size hints may follow on the same line. +- **MIME show full branch — subsequent lines**: indented two spaces for readability. +- **2-arg show signature**: `Base.show(io::IO, x::MyType)` +- **2-arg show content**: one `print` call (no `println`), type name followed by a parenthesised hint matching `Base.summary` style, e.g. `print(io, "Decomposition (10×10)")`. +- **summary signature**: `Base.summary(io::IO, x::MyType)` +- **summary content**: one `print` call (no `println`), type name followed by a parenthesised hint, e.g. `print(io, "ScalarFeature (256 features, Cosine)")`. +- **No collection contents**: print only counts, sizes, or ranges — never iterate and print elements. +- **No allocations**: use `length()`, `size()`, `isempty()`, and `first()` on lazy iterators. Do not call `collect()`, `sort()`, or any function that copies a collection. +- **Tests — MIME full show**: use `sprint(show, MIME("text/plain"), x)` to capture output without side effects. +- **Tests — compact show**: use `sprint(show, MIME("text/plain"), x; context=:compact => true)` to exercise the compact branch, and `sprint(show, x)` to test the 2-arg method directly. +- **Tests — summary**: use `sprint(summary, x)` to capture the one-line description. + +## Examples + +### Example 1 — matrix-carrying type (size hint) + +```julia +# Scenario: a type wraps three matrices — the original, its factorisation, and its +# inverse — used to solve linear systems efficiently during prediction. + +# Before (default Julia show — prints all three matrices in full) +julia> dc +Decomposition(full_matrix=[10×10 Float64 Matrix], decomposition=SVD{Float64,...}(...), + inv_decomposition=[10×10 Float64 Matrix]) + +# After — custom show (two overloads) +function Base.show(io::IO, ::MIME"text/plain", x::Decomposition) + if get(io, :compact, false) + show(io, x) + else + m, n = size(x.full_matrix) + println(io, "Decomposition") + println(io, " size : ", m, " × ", n) + println(io, " method: ", nameof(typeof(x.decomposition))) + end +end + +function Base.show(io::IO, x::Decomposition) + m, n = size(x.full_matrix) + print(io, "Decomposition (", m, "×", n, ")") +end + +# julia> dc +# Decomposition +# size : 10 × 10 +# method: SVD + +# julia> [dc, dc] +# 2-element Vector{Decomposition}: +# Decomposition (10×10) +# Decomposition (10×10) + +# After — custom summary (matches 2-arg show) +function Base.summary(io::IO, x::Decomposition) + m, n = size(x.full_matrix) + print(io, "Decomposition (", m, "×", n, ")") +end +``` + +### Example 2 — composite type with nested objects + +```julia +# Scenario: a type bundles a feature count, an activation function, and a large +# ParameterDistribution of sampled feature parameters — printing it by default +# dumps the entire distribution in full. + +# Before (default Julia show — nested ParameterDistribution printed in full) +julia> sf +ScalarFeature{Cosine}(n_features=256, feature_sampler=Sampler{MersenneTwister}( + parameter_distribution=ParameterDistribution{...}(...), rng=MersenneTwister(...)), + scalar_function=Cosine(), feature_sample=ParameterDistribution{...}(...), + feature_parameters=Dict("sigma"=>1)) + +# After — custom show (two overloads) +function Base.show(io::IO, ::MIME"text/plain", x::ScalarFeature) + if get(io, :compact, false) + show(io, x) + else + println(io, "ScalarFeature") + println(io, " n_features : ", x.n_features) + println(io, " scalar_function: ", nameof(typeof(x.scalar_function))) + if !isnothing(x.feature_parameters) + println(io, " feature_params : ", sprint(summary, x.feature_parameters)) + end + end +end + +function Base.show(io::IO, x::ScalarFeature) + print(io, "ScalarFeature (", x.n_features, " feature", + x.n_features == 1 ? "" : "s", ", ", nameof(typeof(x.scalar_function)), ")") +end + +# julia> sf +# ScalarFeature +# n_features : 256 +# scalar_function: Cosine +# feature_params : Dict{String, Any} with 1 entry + +# julia> [sf, sf] +# 2-element Vector{ScalarFeature{Cosine}}: +# ScalarFeature (256 features, Cosine) +# ScalarFeature (256 features, Cosine) + +# After — custom summary (matches 2-arg show) +function Base.summary(io::IO, x::ScalarFeature) + print(io, "ScalarFeature (", x.n_features, " feature", + x.n_features == 1 ? "" : "s", ", ", nameof(typeof(x.scalar_function)), ")") +end +``` + +### Example 3 — configuration type with regularisation and batch sizes + +```julia +# Scenario: a configuration struct holds a random feature object, a regularisation +# matrix (which may be large), and a batch-size dict. Printing it by default dumps +# all nested objects including the full regularisation matrix. + +# Before (default Julia show — dumps all nested objects) +julia> rfm +RandomFeatureMethod{ScalarFeature{Cosine}}(random_feature=ScalarFeature{Cosine}(...), + batch_sizes=Dict("train"=>0, "test"=>0, "feature"=>0), + regularization=UniformScaling{Float64}(2.220446049250313e-10), + tullio_threading=true) + +# After — custom show (two overloads) +function Base.show(io::IO, ::MIME"text/plain", x::RandomFeatureMethod) + if get(io, :compact, false) + show(io, x) + else + bs = x.batch_sizes + println(io, "RandomFeatureMethod") + println(io, " feature type : ", nameof(typeof(x.random_feature))) + println(io, " n_features : ", get_n_features(x.random_feature)) + println(io, " regularization: ", nameof(typeof(x.regularization))) + println(io, " batch_sizes : train=", bs["train"], + ", test=", bs["test"], ", feature=", bs["feature"]) + println(io, " threading : ", x.tullio_threading) + end +end + +function Base.show(io::IO, x::RandomFeatureMethod) + print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), + ", n=", get_n_features(x.random_feature), ")") +end + +# julia> rfm +# RandomFeatureMethod +# feature type : ScalarFeature +# n_features : 256 +# regularization: UniformScaling +# batch_sizes : train=0, test=0, feature=0 +# threading : true + +# julia> [rfm, rfm] +# 2-element Vector{RandomFeatureMethod{...}}: +# RandomFeatureMethod (ScalarFeature, n=256) +# RandomFeatureMethod (ScalarFeature, n=256) + +# After — custom summary (matches 2-arg show) +function Base.summary(io::IO, x::RandomFeatureMethod) + print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), + ", n=", get_n_features(x.random_feature), ")") +end +``` + +### Unit tests + +```julia +@testset "Decomposition show" begin + dc = Decomposition(rand(8, 8), "svd") + out = sprint(show, MIME("text/plain"), dc) + @test occursin("Decomposition", out) + @test count(==('\n'), out) <= 10 +end + +@testset "Decomposition show compact" begin + dc = Decomposition(rand(8, 8), "svd") + # exercise via the 2-arg method directly + out2 = sprint(show, dc) + @test occursin("Decomposition", out2) + @test !occursin('\n', out2) + # exercise via the MIME method with compact context + out3 = sprint(show, MIME("text/plain"), dc; context = :compact => true) + @test out2 == out3 # both paths must agree +end + +@testset "Decomposition summary" begin + dc = Decomposition(rand(8, 8), "svd") + out = sprint(summary, dc) + @test occursin("Decomposition", out) + @test !occursin('\n', out) # must be exactly one line +end +``` diff --git a/.claude/skills/docstrings/SKILL.md b/.claude/skills/docstrings/SKILL.md new file mode 100644 index 0000000..2e82b8e --- /dev/null +++ b/.claude/skills/docstrings/SKILL.md @@ -0,0 +1,548 @@ +--- +name: docstrings +description: > + Add or normalise Julia docstrings on public symbols (exported types, functions, + and constants) so the package's public API is fully self-documenting and the + Documenter.jl docs build passes its checkdocs check. After writing docstrings, + also updates docs/src/API/ pages so every exported symbol appears exactly once, + organised into logical categories, with stale entries removed. + Invoke this skill whenever the user mentions: docstring, missing doc, + undocumented symbol, API doc, checkdocs warning, docs/src/API, @docs block, + or asks to document a type or function, sync the API pages, or keep the API + index up to date. Also use it when the user asks to "write docs for" or "add + docs to" source files, or when a CI failure mentions missing or incomplete + docstrings. In RandomFeatures.jl, priority targets are: scalar function types + (Cosine, Relu, Gelu, etc. have minimal docstrings), get_output_dim (missing), + and any function whose docstring is a bare $(TYPEDSIGNATURES) with no prose. +--- + +# docstrings + +Add or normalise Julia docstrings on public symbols (exported types, functions, +and constants) across the package source. The goal is complete, consistent API +documentation that renders correctly under Documenter.jl and follows whichever +docstring convention is already established in the package — in RandomFeatures.jl +this is the DocStringExtensions new format: `$(TYPEDEF)` + prose + `$(TYPEDFIELDS)` +for structs, and `$(TYPEDSIGNATURES)` + prose for functions. Completing this skill +makes the package's public API fully self-documenting and satisfies any `checkdocs` +requirement in the docs build. + +## Workflow + +### Step 1 — Detect the existing convention + +Use an Explore subagent to read 2–3 symbols that already have complete docstrings +to calibrate style. This avoids consuming the main context window with large file +reads. Ask the Explore agent to return the verbatim docstring text for each symbol. + +Identify: + +- Whether DocStringExtensions macros are used, and which ones (`$(TYPEDEF)`, + `$(TYPEDFIELDS)`, `$(TYPEDSIGNATURES)`, `$(METHODLIST)`). +- How prose is structured relative to macro-generated content (e.g. does prose + come before or after `$(TYPEDFIELDS)`?). +- What field documentation pattern is preferred: inline string literals above + each struct field vs. a separate prose block. +- Which format is used for struct docstrings: the **old format** (an indented + type-name header on the first line, no `$(TYPEDEF)`, manual `# Constructor` + section), or the **new format** (prose only, `$(TYPEDEF)` for the signature, + `$(METHODLIST)` for constructors). Normalise old-format structs to new-format + during Step 3. + +This detected baseline becomes the style target for every new or normalised +docstring. Do not impose a different convention — match what is already there. + +### Step 2 — Enumerate candidates + +Discover the package name from `Project.toml` (the `name =` field). Then run: + +``` +grep -nE '^(function |struct |abstract type |mutable struct |const )' src/**/*.jl +``` + +**Cross-file exports**: exported names may be declared in a central module file +(e.g. `src/PackageName.jl`) while the definition lives in a different file. +Read the module file for all `export` statements so you catch every public symbol +regardless of where it is defined. + +For each exported symbol, check whether a non-empty docstring immediately precedes +the definition. Produce a prioritised list: + +1. **Missing entirely** — no docstring at all. +2. **Old-format struct** — indented type name on first line, no `$(TYPEDEF)`, + or redundant manual `# Constructor` / `# Constructors` section alongside + `$(METHODLIST)`. +3. **Empty or stub** — only a bare macro line (e.g. `$(TYPEDSIGNATURES)`) with + no prose. +4. **Incomplete** — prose present but key sections absent (missing `# Arguments`, + `# Examples`, or field strings not describing semantic role). + +**Also scan every function in the file for old-style docstrings**, regardless of +whether it is exported. An old-style docstring is one that uses an indented +function-name header (e.g. ` my_func(arg1, arg2)`) and/or an `Args:` / +`Arguments:` block with the `` `name` - description `` format. Convert these to +the `$(TYPEDSIGNATURES)` convention in the same editing pass — the whole file +should end up stylistically uniform. + +### Step 3 — Draft docstrings + +For each candidate, write a docstring that matches the detected convention. + +#### Getters and simple accessors + +Simple getters for exported types (e.g. `get_n_features(rf::RandomFeature)`, +`get_rng(s::Sampler)`) are public API and must be documented if exported. A +one-line `$(TYPEDSIGNATURES)` + short prose sentence suffices; no `# Arguments` +or `# Examples` is needed unless the semantics are non-obvious. + +#### Old-format struct docstrings + +If a struct docstring starts with an indented type name (e.g. ` MyStruct{...}`), +convert it to the new format: + +- Remove the indented type name from the docstring. +- Add `$(TYPEDEF)` immediately after the opening prose sentence. +- Replace any manual `# Constructor` or `# Constructors` section (listing + function signatures) with a `# Constructors` section containing only + `$(METHODLIST)`. If `$(METHODLIST)` is already present alongside the manual + list, remove the manual list. +- Preserve any genuine prose that was in the old `# Constructor` section if it + explains non-obvious behaviour; discard boilerplate signature repetition. + +#### Named constructors (factory functions) + +`$(METHODLIST)` only lists methods whose name matches the struct type. Exported +functions that **build an instance of the struct but carry a different name** — +factory functions such as `FeatureSampler` for `Sampler`, or `ScalarFourierFeature` +and `ScalarNeuronFeature` for `ScalarFeature` — are invisible to `$(METHODLIST)` +and must be surfaced manually in the struct docstring. + +When such functions exist, add a prose note inside the `# Constructors` section, +immediately before `$(METHODLIST)`: + +```julia +""" +Wrap the parameter distributions used to sample random features. + +$(TYPEDEF) + +# Fields + +$(TYPEDFIELDS) + +# Constructors + +Recommended construction is via the `FeatureSampler()` utility, which assembles +the sampler from a feature distribution and an optional bias distribution. +Convenience factories `ScalarFourierFeature` and `ScalarNeuronFeature` build the +full `ScalarFeature` (including its sampler) in a single call. + +$(METHODLIST) +""" +struct Sampler{RNG <: AbstractRNG} + ... +end +``` + +The note should: +- Name the function in backticks. +- Give a one-line hint about when to prefer it over the direct constructor. +- Optionally include a minimal usage snippet if the factory is the primary entry + point and no separate `# Examples` block exists on the factory function itself. + +The factory function still needs its **own full docstring** (`$(TYPEDSIGNATURES)`, +`# Arguments`, `# Examples` if non-trivial). The struct-level note is a pointer, +not a replacement. + +**Detecting named constructors during Step 2:** when enumerating candidates, flag +exported functions whose name differs from any type name but whose body or doc +clearly returns an instance of a specific struct. Common signals: the function name +ends with a domain term (e.g. `ScalarFourierFeature`, `FeatureSampler`), its return +statement calls the struct constructor directly, or the existing codebase already +mentions the relationship somewhere in prose. + +#### Multiple dispatch — one docstring per concept + +When a function has multiple dispatch methods, document **only** the primary +user-facing overload and leave all other overloads undocumented. Competing +docstrings fragment the rendered API docs and create maintenance burden. + +The primary overload is the method whose argument type is the **broadest +user-facing type** — e.g. `RandomFeature` rather than `ScalarFeature` or +`VectorFeature`. + +**Type-parameter specialisations count as overloads.** If the same function name +is defined for multiple concrete feature types, these are dispatch methods of the +same concept. Document only one — typically the first defined, or the more general +one — and leave the rest undocumented. + +Internal framework methods that are exported for extensibility but not called +directly by end users do not need documentation even if exported. + +#### Old-style function docstrings (all functions, not just exported) + +Convert any docstring that uses an indented function-name header or an `Args:` / +`Arguments:` section to the `$(TYPEDSIGNATURES)` style, even for internal +(non-exported) helpers. The canonical old-style markers are: + +- First line indented with spaces: ` my_func(arg, ...)` — replace with `$(TYPEDSIGNATURES)`. +- Argument block labelled `Args:` or `Arguments:` with `` `name` - description `` lines — replace + with a `# Arguments` section using `` - `name`: description `` format. + +Doing this in the same pass keeps the file stylistically uniform and prevents +old-style docstrings from persisting as invisible technical debt. + +#### General rules + +- Use the same macro set as the best-documented symbols already in the package. +- Preserve any inline field string literals already present above struct fields — + do not merge them into the struct-level docstring. +- Prose should answer: what does this symbol represent or do, when would a caller + use it, and what are the physical units of key quantities. +- Do not duplicate content that macros generate automatically (e.g. do not + restate field types when `$(TYPEDFIELDS)` already renders them). +- Physical quantities: always include units in square brackets, e.g. `[m/day]`. +- For functions with more than two arguments, or whose argument semantics are + not obvious from the name alone, add a `# Arguments` section listing each + parameter as `` - `name`: description [unit if applicable] ``. +- For every non-trivial public function where a minimal runnable example can be + written, add a `# Examples` section with a `jldoctest` block so Documenter.jl + can verify the example stays correct as the code evolves. + +### Step 4 — Apply edits + +When editing files that contain non-ASCII characters (e.g. author names with +accented letters), the file may store characters in Unicode NFD form while the +Edit tool normalises to NFC, causing match failures. If an Edit call fails with +a "not found" error on a string you can see in the file, use a Python one-liner +to apply the replacement with NFD-normalised strings: + +```bash +python3 - <<'EOF' +import unicodedata, pathlib +p = pathlib.Path("src/MyFile.jl") +text = p.read_text() +old = unicodedata.normalize('NFD', "the old string here") +new = unicodedata.normalize('NFD', "the new string here") +p.write_text(text.replace(old, new, 1)) +EOF +``` + +After a Python edit, re-read the file before making any further Edit calls to +the same file (the Edit tool tracks file state from the last Read). + +### Step 5 — Sync `docs/src/API/` pages + +After all source-file edits are applied, update the Documenter.jl API pages so +that every exported, documented symbol appears exactly once, organised into +logical categories. The goal is that a reader browsing `docs/src/API/` sees a +complete, non-redundant index of the public API — nothing missing, nothing +stale. + +#### 5a — Build the source-to-page map + +Read `docs/make.jl` and extract the `api` array to see which display name maps +to which page path (e.g. `"Methods" => "API/Methods.md"`). For each page, +read its `@meta` block to find `CurrentModule = ...`. This tells you which +module's exports the page is responsible for. + +#### 5b — Collect current `@docs` entries per page + +For each API page, extract every symbol entry listed inside ` ```@docs ``` ` +blocks. Some entries carry type-signature qualifiers (e.g. +`get_n_features(rf::ScalarFeature)`) — track both the raw entry string and +the base name (everything before the first `(`). + +#### 5c — Find missing and stale entries + +**Exported but not defined (phantom exports).** Before anything else, check +that every exported name actually resolves to a definition — a function, type, +or constant — somewhere in the source files of that module. If an exported name +has no definition anywhere, it is a phantom export: remove the `export` +statement (or just that name from a multi-name `export` line) from the source +file. Do not add phantom exports to any API page. + +**Missing from the API.** A symbol is **missing** from a page when it is +exported from that page's `CurrentModule`, its base name does not appear in any +`@docs` block on any API page, and it has a definition in the source. If it +lacks a docstring, go back and write one now (following the conventions from +Steps 1–3) before adding it to the API page — an undocumented entry in a +`@docs` block will cause the docs build to error. Every exported, defined +symbol must end up with a docstring and an API page entry. + +**Stale API entries.** An entry is **stale** when the base name is no longer +exported from the module, or the symbol no longer has a definition in the +source. + +Run all three checks before making edits so you can see the full diff in one +pass. + +#### 5d — Place missing symbols into appropriate sections + +Insert each missing symbol into the section of its API page that best matches +its role. Use the existing section headings on the page as the primary guide — +`## Getter functions`, `## Error metrics`, etc. are already established +categories; add the new symbol to the most thematically fitting one. + +When no existing section fits, create a new `##` heading that names the +functional group (e.g. `## Factory functions`, `## Utility functions`) and open +a fresh ` ```@docs ``` ` block below it. Avoid catch-all sections like +`## Miscellaneous`; if you find yourself reaching for that, split more finely. + +Broad heuristics for categorisation when the page has no existing sections to +guide you: + +- Struct / abstract type → primary types section (first block on page) +- Functions starting with `get_` → `## Getter functions` +- Functions starting with `compute_`, `construct_`, `build_` → a computation + or construction section +- Factory functions (e.g. `ScalarFourierFeature`, `FeatureSampler`) → `## Factory functions` +- Update or step functions → an operations section +- Error-metric functions → `## Error metrics` + +For a multiple-dispatch function where only the primary overload is documented +(per Step 3), list only that overload. If the existing page convention uses +type-qualified entries (e.g. `get_n_features(rf::RandomFeature)`), follow that +convention; otherwise use the plain name. + +#### 5e — Remove stale entries + +For each stale API entry: + +1. Delete the line from its `@docs` block. If that empties the block, delete + the block. If that empties the section, delete the section heading too. +2. If the symbol is stale because it is no longer defined (phantom export), + also remove the `export` statement from the source file. For multi-name + export lines (e.g. `export foo, bar, baz`), remove only the stale name and + leave the rest intact. + +#### 5f — Ensure no symbol appears on two pages + +Each base name must appear on at most one API page. If you find a duplicate, +keep it on the page whose `CurrentModule` matches the module where the symbol +is defined, and remove it from the other page. + +### Step 6 — Verify + +Find the package name from `Project.toml`, then confirm the package loads +without error: + +``` +julia --project -e 'import Pkg; Pkg.instantiate(); using RandomFeatures' +``` + +If a docs build is configured (`docs/make.jl` is present), run it and resolve +any `checkdocs` warnings introduced by the new docstrings. + +### Step 7 — Offer to improve the skill + +Once the docs build is clean, ask the user: "Would you like to improve the +**docstrings** skill itself using skill-creator? You can share suggestions, or I +can analyse patterns from this session — recurring edge cases, formatting +decisions, or anything that felt awkward — to refine the skill for next time." + +## Formatting rules + +These rules encode the conventions most Julia packages following DocStringExtensions +expect. Apply them consistently. + +- **Triple-quoted strings** for all docstrings. +- **First line**: concise one-line summary — imperative mood for functions + (`"Return the..."`, `"Compute..."`), noun phrase for types and constants. +- **Second line**: blank. +- **Body**: prose, then any macro invocations. `$(TYPEDSIGNATURES)` must be the + very first line of a function docstring and is the sole source of the method + signature — never write a manual indented signature as well. +- **No trailing whitespace** inside the docstring. +- **No emojis.** +- **Physical units** in square brackets: `[m/day]`, `[kg/m³]`, `[day]`, etc. +- **Field string literals** (the string above each struct field) are distinct + from the struct-level docstring. Preserve both; do not merge them. +- Field string literals must describe the field's *semantic role*, not its type. + Never write a type name inside brackets (e.g. `"[Dict]"`) — + `$(TYPEDFIELDS)` already renders the type. Reserve square-bracket notation + exclusively for physical units. +- Avoid vague labels such as "data object" or "container". Say what the field + represents in domain terms (e.g. "number of random features used in the + approximation" rather than "integer count"). +- **Multiple-dispatch — one docstring per concept**: Document only the primary + user-facing overload (the method taking the top-level composite type). All + other dispatch methods remain undocumented. Do **not** add `$(METHODLIST)` to + function docstrings — `$(TYPEDSIGNATURES)` already surfaces all overloads. + `$(METHODLIST)` belongs only in struct docstrings (inside `# Constructors`). +- **`# Arguments` section**: add after the opening prose for any function with + more than two parameters, or where argument semantics are non-obvious. Format: + `` - `name`: description [unit] ``. +- **`# Examples` section**: add for every non-trivial public function where a + minimal runnable example is feasible. Use `jldoctest` blocks with `julia> ` + prompts and include expected output. +- In every `jldoctest` block, separate each `julia> ` prompt from the next with + a blank line. Documenter.jl rejects blocks where two prompts appear + consecutively without an intervening blank line. If a statement produces no + output, end it with a semicolon and add a blank line before the next prompt. +- If the doctest references any name from the package, the first statement must + be `julia> using RandomFeatures` (followed by a blank line). Do not assume the + package is already in scope. + +## Quality criteria + +| Criterion | Weight | What to check | +|---|---|---| +| **Completeness** | High | Every exported symbol has a non-empty docstring after the task is applied. | +| **Convention parity** | High | New docstrings use the same macro set and structural pattern as the best-documented symbols already present. Old-format struct docstrings have been normalised. | +| **Informativeness** | Medium | Prose answers "what, when, why". Units present for physical quantities. `# Arguments` section present where needed. `# Examples` jldoctest block present for non-trivial public functions. | +| **No duplication** | Medium | Prose does not duplicate macro-generated content. Field string literals do not restate the field's type. No redundant manual `# Constructor` section alongside `$(METHODLIST)`. | +| **API page coverage** | High | Every exported, documented symbol appears exactly once across `docs/src/API/` pages. No stale entries. Symbols are grouped into descriptive sections. | +| **Correctness** | High | Package loads without error; docs build (if configured) completes without new warnings. | + +## Examples + +### Struct: old format → new format + +```julia +## Before — OLD format: indented type name, no $(TYPEDEF), manual # Constructor section + +""" + Fit{V<:AbstractVector, USorM<:Union{UniformScaling, AbstractMatrix}} + +Holds the fitted random feature coefficients and matrix decomposition. + +# Constructor +Fit(feature_factors, coeffs, regularization) + +# Fields + +$(TYPEDFIELDS) + +# Constructors + +$(METHODLIST) +""" +struct Fit{V <: AbstractVector, USorM <: Union{UniformScaling, AbstractMatrix}} + ... +end + +## After — NEW format: prose only, $(TYPEDEF) for signature, $(METHODLIST) for constructors + +""" +Hold the coefficients and matrix decomposition that describe a set of fitted random features. + +Instances are produced by `fit(rfm, inputs, outputs)` and passed to `predict` and +`predictive_mean` / `predictive_cov` to generate predictions at new input locations. + +$(TYPEDEF) + +# Fields + +$(TYPEDFIELDS) + +# Constructors + +$(METHODLIST) +""" +struct Fit{V <: AbstractVector, USorM <: Union{UniformScaling, AbstractMatrix}} + ... +end +``` + +### Function: stub docstring improved + +```julia +## Before — function with an empty stub docstring + +""" +$(TYPEDSIGNATURES) +""" +function Decomposition(mat::AbstractMatrix, method::String; nugget::Real = 1e12 * eps()) + ... +end + +## After — prose, Arguments, and Examples sections added + +""" +$(TYPEDSIGNATURES) + +Compute and store a matrix factorisation of `mat` using `method`, together with its +inverse, for efficient repeated linear solves. + +# Arguments +- `mat`: the square matrix to factorise. +- `method`: factorisation strategy — one of `"svd"`, `"cholesky"`, or `"pinv"`. +- `nugget`: small regularisation constant added to the diagonal before factorising [`Float64`]. + +# Examples +```jldoctest +julia> using RandomFeatures + +julia> M = [4.0 2.0; 2.0 3.0]; + +julia> dc = Decomposition(M, "cholesky"); + +julia> size(get_decomposition(dc)) +(2, 2) +``` +""" +function Decomposition(mat::AbstractMatrix, method::String; nugget::Real = 1e12 * eps()) + ... +end +``` + +### Multiple-dispatch: type-parameter specialisations + +```julia +## Before — both specialisations documented (anti-pattern) + +""" +$(TYPEDSIGNATURES) + +Return the predictive mean for a scalar random feature model. +""" +function predictive_mean(rfm::RandomFeatureMethod, inputs::AbstractMatrix, + fitted::Fit; kwargs...) # ScalarFeature path + ... +end + +""" +$(TYPEDSIGNATURES) + +Return the predictive mean for a vector random feature model. +""" +function predictive_mean(rfm::RandomFeatureMethod, inputs::AbstractMatrix, + fitted::Fit; kwargs...) # VectorFeature path + ... +end + +## After — only the first overload documented; second left bare + +""" +$(TYPEDSIGNATURES) + +Return the predictive mean of the fitted random feature model evaluated at `inputs`. +""" +function predictive_mean(rfm::RandomFeatureMethod, inputs::AbstractMatrix, + fitted::Fit; kwargs...) + ... +end + +function predictive_mean(rfm::RandomFeatureMethod, inputs::AbstractMatrix, + fitted::Fit; kwargs...) + ... +end +``` + +### Simple getter: minimal docstring + +```julia +## Before — getter with no docstring + +get_n_features(rf::RandomFeature) = rf.n_features + +## After — one-liner is enough + +""" +$(TYPEDSIGNATURES) + +Return the number of random features in `rf`. +""" +get_n_features(rf::RandomFeature) = rf.n_features +``` diff --git a/.claude/skills/error-message-manager/SKILL.md b/.claude/skills/error-message-manager/SKILL.md new file mode 100644 index 0000000..7a0841e --- /dev/null +++ b/.claude/skills/error-message-manager/SKILL.md @@ -0,0 +1,818 @@ +--- +name: error-message-manager +description: > + Rewrite vague, delayed, or low-context Julia error messages into structured, + actionable diagnostics. Invoke this skill whenever the user mentions: error + message, improve errors, rewrite @assert, ArgumentError, DimensionMismatch, + DomainError, vague error, error rewrite, Julia exception, diagnostic, throw, + validation, early check, assert to throw, loop context, catch and rethrow, + warn string, or asks to improve how the code fails. Also use it when reviewing + code for user-facing clarity, when a user says errors are confusing or + unhelpful, or when auditing a module for low-context exceptions. Use it + proactively when you see bare @assert, error("..."), throw(ErrorException(...)), + @warn string(...), or catch blocks that do not include the original exception + in their re-throw in Julia code you are reading or editing. +--- + +# error-message-manager + +Rewrite vague, delayed, or low-context Julia error messages into structured, +actionable diagnostics. The goal is errors that tell the user exactly what went +wrong, what was expected, what was received, and—whenever a likely fix exists— +what to do next. Prefer catching mistakes early (at API boundaries) over letting +them propagate into cryptic numerical failures. + +## Workflow + +### Step 0 — Offer an Explore agent for multi-file scope + +If the user's request covers more than one file — a whole directory, a module, or +the entire repo — offer to spawn an Explore agent before doing any file reads +yourself. The agent runs all the reads in parallel without flooding the main +context, and returns a structured inventory you can act on directly. + +**When to offer**: any time the target is a directory path (e.g. `src/`) or a +vague scope like "the whole package" or "all the source files". + +**Offer text** (adapt as needed): +> "This spans multiple files — I'd recommend spawning an Explore agent to survey +> all `throw`/`@assert`/`error` sites in parallel. It keeps the audit fast and +> leaves the main context clean for the actual rewrites. Want me to do that?" + +**Agent prompt to use** (fill in `` and ``): + +``` +Audit `` for error-raising patterns. For every `@assert`, `error(`, or +`throw(` site in every `.jl` file: + +1. Record: file, line number, exception type (or "bare @assert" / "bare error"), + and the full message text (including multiline strings). +2. Classify message quality: + - "good" — has `$(expr)` interpolation showing the actual received value, and + is either short (≤3 lines) or already in a `_throw_` helper function + - "long-inline" — message content is good, but the body exceeds 3 lines and + the throw is written inline (not in a `_throw_` helper) + - "vague" — missing a received value, or no Expected/Got structure + - "missing" — bare `@assert` with no message at all +3. Note whether the site is at an API boundary (user-facing input) or an internal + invariant (would require a package bug to fire). + +Return a markdown table with columns: + File | Line | Exception type | Quality | Notes (one-line note on what's wrong if vague/missing/long-inline) + +Focus only on sites that are "vague", "missing", or "long-inline" — skip "good" ones. +``` + +**How to use the result**: treat the returned table as your working inventory for +Steps 1 and 2. You do not need to re-read the flagged files yourself to classify — +go straight to reading only the lines that need rewrites (Step 3 onwards). + +--- + +### Step 1 — Audit the target scope + +Identify which files or functions to address. If the user named a specific +function, start there. If the request is repo-wide, run: + +``` +rg -n '(@assert[^(]|@assert\(|error\(string\(|throw\(ErrorException)' src/ +``` + +Then collect all message-less `@assert` calls: + +``` +rg -n '@assert' src/ | grep -v '"' +``` + +Also flag `@warn` calls that use string concatenation instead of interpolation: + +``` +rg -n '@warn\s+string\(' src/ +``` + +And flag `catch` blocks that discard the original exception when re-throwing: + +``` +rg -n 'catch\s' src/ +``` + +For each `catch` hit, check whether the subsequent `throw` or `error` call +interpolates the caught variable (e.g. `$e` or `sprint(showerror, e)`). If it +does not, the original exception type and message are silently lost. + +For each hit, record: file, line, the condition being checked, and whether it +guards user-provided input (API boundary) or an internal invariant. + +### Step 2 — Classify each site + +Use this table to choose the right exception type: + +| Condition | Exception | +|---|---| +| Invalid user-provided argument | `ArgumentError` | +| Array/matrix shape mismatch | `DimensionMismatch` | +| Inconsistent argument types across parameters | `ArgumentError` | +| Mathematically invalid value (negative variance, etc.) | `DomainError` | +| Invalid index | `BoundsError` | +| Internal invariant that should never fire | `error(...)` | +| Missing interface implementation | `MethodError` or structured `ArgumentError` | + +Avoid `ErrorException` unless there is no better choice. + +**Type mismatches vs dimension mismatches**: a check that the user supplied +*consistent* arguments (e.g. a feature sampler whose distribution contains the +required "xi" parameter) is checking argument consistency, not size matching. +Use `ArgumentError`, not `DimensionMismatch`, for this pattern. + +Distinguish **API boundary** sites (where the user passed something wrong — prefer +typed exceptions with actionable messages) from **internal invariant** sites +(where a bug in the package itself would have to exist — bare `error(...)` with a +clear note is fine there). + +**Loop-body errors**: if the throw is inside a `for` or `while` loop, treat the +loop index and key per-iteration state as required context. Without this, the user +sees "matrix is not positive definite" with no idea whether it happened on +training batch 2 or batch 200. Always capture `i` (or the loop variable) and the +state that changed between iterations — the batch index, the feature matrix being +decomposed, etc. For *nested* loops, include both the outer and inner loop +variables. See the loop-context example in the Canonical examples section below. + +**`catch e` losing the original exception**: when a Julia exception is caught and +a new one is thrown, the new message must include the original exception. If it +does not, the user loses the root cause (e.g. `PosDefException`, `SingularException`) +and has no way to distinguish a code bug from a numerical issue. Use +`sprint(showerror, e)` rather than `$e` alone — it formats the exception type and +message together: + +```julia +# anti-pattern — root cause vanishes +catch e + throw(ArgumentError("Matrix factorization failed.")) +end + +# correct — root cause preserved +catch e + throw(ArgumentError(""" +Matrix factorization failed. + +Caused by: $(sprint(showerror, e)) + +Suggestion: + ... +""")) +end +``` + +Only suppress the original exception if it is a well-known internal Julia error +(e.g. `SingularException`) and you are intentionally providing a higher-level +fallback — and even then, log it at `@debug` level. + +**`@warn string(...)` concatenation and broken interpolation**: two anti-patterns: +`@warn string("...", x, "...")` and `@warn "... cond(regularization) ..."` (a +template string that never interpolates the actual value). Both are equivalent — +the user sees a static message with no actual numbers. Rewrite as +`@warn "... $(cond(regularization)) ..."`. Interpolated `@warn` messages are also +easier to grep and suppress selectively. + +**Double-gated invariants**: if a helper is only ever called after the public API +has already checked the same condition, the check inside the helper is an internal +invariant even though it looks like a user-data check. Use a single-line `error(...)` +rather than a full structured `ArgumentError`: + +```julia +# internal invariant — the caller already validated this +haskey(params, "xi") || error( + "Internal error: feature builder called without 'xi' distribution (caller should have validated)", +) +``` + +### Step 2.5 — Decide: inline or helper? + +Before writing the rewrite, decide whether the error belongs inline or should be +extracted into a `_throw_(...)` helper function. + +**When to extract** — pull the error into a helper when either condition holds: + +- **Length** (primary trigger): the message body exceeds 3 lines. Extract + unconditionally — single call site, non-loop context, no surrounding complexity + required. A full Expected / Got / Suggestion block always crosses this threshold. + Even a one-off long block left inline establishes a pattern that makes entire files + hard to scan, and accumulates quickly once a few exceptions are made. +- **Duplication**: the same error shape (same summary line, same Expected / Got / + Suggestion skeleton) appears at ≥2 call sites. Extract even when each block is + short — the wording drifts silently over time and the call sites collapse to + readable one-liners. + +Inline is appropriate only for genuinely short messages (≤3 lines) at a single +call site. A single summary line, or a summary plus one Got line, is the ceiling +for inline. When in doubt, count — if it doesn't fit in 3 lines, extract. + +**Where helpers go** + +Default: a `## Error helpers` section at the **bottom of the source file**, above +`end # module`. Keeping helpers near their callers preserves traceability — the +reader sees the throw site, jumps to the bottom of the same file, and finds the +message without switching files. + +Promote to a shared `src/ErrorMessages.jl` (or the repo's equivalent top-level +utility file) only when **≥2 different source files** call the same helper. Discover +which file to use by reading the top-level module file (e.g. `src/PackageName.jl`) +for its `include(...)` list — then add `include("ErrorMessages.jl")` as the first +`include` so every subsequent file sees the helpers without any `using`/`import`. + +**Naming convention** + +``` +_throw_(positional_required_facts...; kwargs_for_optional_context...) +``` + +- Underscore prefix → unexported private helper. +- Verb prefix `_throw_` → the function unconditionally raises; callers know there + is no return value. +- Suffix describes the failure mode: `_dim_mismatch`, `_missing_xi`, + `_bad_method`, `_not_iterable`. + +**Signature convention** + +Pass the facts that are *always* present as positional arguments (the offending +value, the expected vs got summary). Pass *optional* context as keyword arguments +with `nothing` defaults — especially loop context (`index`, `total`, `batch`). +Build optional sections inside the helper by checking `isnothing(...)`. +This keeps call sites compact and lets the same helper serve both loop and non-loop +contexts (see the *Helper with optional loop context* canonical example). + +**Performance: use `@noinline`** + +Prefix every helper with `@noinline`. This prevents Julia from inlining the cold +error path into the surrounding hot code, keeping numerical kernels unaffected: + +```julia +@noinline function _throw_missing_xi(pd; where::Symbol) + throw(ArgumentError(...)) +end +``` + +**What NOT to do** + +- Don't create a catch-all `_throw_arg_error(msg::String)` — that just shifts the + inline triple-quoted block to another file without any DRY benefit. +- Don't use macros (`@check_xi(...)`) — they're magical and harder to debug than + plain functions. +- Don't bundle all context into one opaque `context::NamedTuple` — explicit kwargs + are clearer to call and easier to extend. + +### Step 3 — Rewrite with the canonical layout + +Use this structure for every user-facing exception: + +```julia +throw(ArgumentError(""" +Short one-line summary of the failure. + +Expected: + + +Got: + + +Loop context: + iteration = $iter (of $n_iter) + = $(summary_of_state) + +Context: + + +Suggestion: + +""")) +``` + +Section rules: +- **Summary**: always present; one line; imperative or declarative. +- **Expected / Got**: strongly preferred for any mismatch check; use `$(expr)` + interpolation to show actual values. +- **Loop context**: include whenever the throw is inside a `for` or `while` loop. + Always report the loop index and the key state that varies between iterations + (e.g., the batch number, the feature matrix being decomposed). This is what lets + the user reproduce the failure without adding `println` debugging. +- **Context**: include when the same error can arise from multiple call sites and + naming the calling function or struct helps the user orient. +- **Suggestion**: include whenever a likely fix exists. Omit rather than write a + generic platitude. +- Never dump full matrices or large arrays. Prefer `size(x)`, `eltype(x)`, + `typeof(x)`, or a scalar summary statistic. + +### Step 4 — Move validation early + +If the current code lets an invalid input reach a numerical routine before +failing (e.g., `cholesky` on a non-positive-definite matrix, regularization that +is poorly conditioned), add an explicit guard at the API boundary: + +```julia +# Before: error surfaces deep in cholesky during predict +cov_chol = cholesky(C) + +# After: check at the boundary, raise immediately +isposdef(C) || throw(ArgumentError(""" +Feature covariance matrix must be positive definite. + +Got: + size(C) = $(size(C)) + isposdef(C) = false + minimum eigval ≈ $(minimum(eigvals(Symmetric(C)))) + +Suggestion: + Increase the regularization parameter in RandomFeatureMethod. +""")) +cov_chol = cholesky(C) +``` + +Use `||` for single-condition guards. For multi-condition guards, use `if/throw`. + +When using `||` with a multiline triple-quoted throw, the closing `))` goes on its +own line immediately after the closing `"""`: + +```julia +condition || throw(ArgumentError(""" +Summary line. + +Expected: + ... + +Got: + ... +""")) # ← closing )) on the line right after the closing """ +``` + +This is the only layout that keeps indentation correct — triple-quoted strings in +Julia do not strip leading whitespace, so indenting the message body would include +those spaces in the string. + +### Step 5 — Preserve domain language + +Write messages in terms the user understands, not in terms of internal Julia +dispatch or linear algebra internals. For example: + +- Say "number of random features" not "size(x, 1)" +- Say "regularization matrix" not "the second argument to cholesky" +- Say "feature parameter distribution" not "the ParameterDistribution object" + +### Step 6 — Apply rewrites + +Edit each site, keeping the surrounding code untouched. Confirm the package +still loads: + +``` +julia --project -e 'using RandomFeatures' +``` + +### Step 7 — Add @test_throws tests + +Before writing any test, check whether coverage already exists. Grep the matching +`test//runtests.jl` for the public API function that reaches the rewritten +site: + +```bash +grep -n '@test_throws' test//runtests.jl | grep '' +``` + +Three outcomes: + +| Situation | Action | +|---|---| +| `@test_throws ` already present | Skip — do not add a duplicate | +| `@test_throws ` already present | Update the existing line to the new type | +| No coverage at all | Add a new test | + +For every site that needs a new test, add it in the matching `test//runtests.jl`: + +```julia +@test_throws ArgumentError ScalarFeature(100, bad_sampler, Cosine()) +``` + +Use the specific exception type — never bare `@test_throws Exception`. The test +should construct the minimal invalid input that triggers the new error, without +duplicating happy-path coverage. + +**Update existing tests that used the wrong type.** If the file already has a +`@test_throws ErrorException` (or any other type) for a site you're rewriting to +`ArgumentError`, update that existing test in the same edit. + +**Testing unexported helpers.** If the site is inside an unexported helper, +do not `import` the internal directly. Instead, test through the nearest exported +public API function that calls it, using invalid input that propagates to the helper: + +```julia +# The "xi" check is inside ScalarFeature's constructor — test via the exported constructor +pd_without_xi = constrained_gaussian("theta", 0.0, 1.0) # no "xi" component +@test_throws ArgumentError ScalarFeature(100, FeatureSampler(pd_without_xi), Cosine()) +``` + +This keeps tests coupled to the public contract and avoids brittleness when +internal function names change. + +### Step 8 — Offer to improve the skill + +Once the rewrites and tests are clean, offer: "Would you like to improve the +**error-message-manager** skill itself using skill-creator? You can share +suggestions, or I can analyse patterns from this session—recurring edge cases, +exception-type decisions, or anything that felt awkward—to refine the skill for +next time." + +--- + +## Style rules + +- **Triple-quoted strings** for all multiline messages. +- **No full matrix dumps**. Use `size(x)`, `eltype(x)`, `norm(x - ...)`, or + `extrema(x)` instead. +- **Interpolate actual values** in Got sections so the user sees the numbers, + not just variable names. For `String`-typed arguments use `$(repr(x))` rather + than `$(x)` — it adds the surrounding quotes so the output clearly reads as a + string value (e.g. `Got: method = "bad"` instead of `Got: method = bad`). +- **Raise early**: prefer guarding at the function entry point over deep inside a + helper. +- **No `@assert` for user-facing validation**. `@assert` is a debugging tool; + it can be compiled out. Use explicit `throw` instead. +- **Loop state in Got / Loop context**: when a throw is inside a `for` or `while` + loop, always name the iteration index and the key state from that iteration. + A "factorization failed" message without the batch index forces the user to add + `println` debugging to reproduce the failure. When the same loop-context check + recurs at multiple sites, the loop variables become optional kwargs on a + `_throw_` helper. +- **Preserve the original exception in `catch` blocks**: if you catch `e` and + throw a new exception, include `$(sprint(showerror, e))` in the new message. + Dropping `e` silently discards the root cause. +- **`@warn` with interpolation, not `string()`**: replace + `@warn string("κ(reg) = cond(regularization)")` with + `@warn "κ(reg) = $(cond(regularization))"`. Uninterpolated template strings + in warnings show only the literal variable name, not its value. +- **Single-line messages are fine** when the failure is unambiguous and no + Expected/Got context would add clarity. +- **Extract into `_throw_(...)` helpers** whenever the message body exceeds + 3 lines, or when the same Expected / Got / Suggestion skeleton appears at ≥2 + call sites (even if short). A full Expected / Got / Suggestion block always + exceeds 3 lines and must be a helper — inline is only appropriate for ≤3-line + messages at a single call site. Place the helper in a `## Error helpers` section + at the bottom of the source file; promote to a shared `src/ErrorMessages.jl` only + when ≥2 different source files share the helper. Use `@noinline`, positional args + for required facts, and `nothing`-defaulted kwargs for optional context such as + loop indices. Render each optional section only when its kwarg is non-`nothing`. + +--- + +## Canonical before/after examples + +> **Length rule applies to all examples below.** Each example shows the canonical +> message *format* (Expected / Got / Suggestion sections, interpolation, etc.). When +> the message body exceeds 3 lines — which any structured block with Expected / Got / +> Suggestion sections does — the throw must go in a `_throw_(...)` helper per +> Step 2.5, not inline. The first example below models this explicitly. Subsequent +> examples show the message body format; apply the same helper extraction whenever +> the resulting message exceeds 3 lines. + +### Replace a vague ArgumentError (missing received values) + +The after-message has 10 lines (above the 3-line threshold), so it goes into a +`_throw_` helper — extract unconditionally at this length even though there is +only one call site. + +```julia +# Before — no interpolation: user sees no names from their distribution +if "xi" ∉ get_name(get_parameter_distribution(feature_sampler)) + throw( + ArgumentError( + " Named parameter \"xi\" not found in names of parameter_distribution. " * + " \n Please provide the name \"xi\" to the distribution used to sample the features", + ), + ) +end + +# After — helper in the ## Error helpers section at the bottom of the file +@noinline function _throw_missing_xi(pd; where::Symbol) + throw(ArgumentError(""" +$where: parameter distribution must include a component named "xi" for feature sampling. + +Expected: + "xi" ∈ get_name(parameter_distribution) + +Got: + available names = $(get_name(pd)) + +Suggestion: + Add a ParameterDistribution component named "xi" when constructing FeatureSampler, + e.g. via constrained_gaussian("xi", μ, σ). +""")) +end + +# Call site collapses to a single guard line: +"xi" ∈ get_name(get_parameter_distribution(feature_sampler)) || + _throw_missing_xi(get_parameter_distribution(feature_sampler); where = :ScalarFeature) +``` + +### Replace a bare `@assert` on an API boundary + +```julia +# Before +@assert(haskey(batch_sizes, "train") && haskey(batch_sizes, "test") && haskey(batch_sizes, "feature")) + +# After +all(k -> haskey(batch_sizes, k), ("train", "test", "feature")) || throw(ArgumentError(""" +batch_sizes dict is missing required keys. + +Expected keys: + "train", "test", "feature" + +Got keys: + $(collect(keys(batch_sizes))) + +Suggestion: + Provide all three keys: Dict("train" => 0, "test" => 0, "feature" => 0). +""")) +``` + +### Replace a string-value error with `repr` + +```julia +# Before — user sees method = svdd with no quotes, hard to see it's a string typo +throw(ArgumentError( + "Only factorization methods \"pinv\", \"cholesky\" and \"svd\" implemented. got " * string(method) +)) + +# After — repr keeps quotes visible, making string typos obvious +throw(ArgumentError(""" +Unrecognised matrix factorisation method. + +Expected: + "pinv", "cholesky", or "svd" + +Got: + method = $(repr(method)) +""")) +``` + +Using `repr(method)` rather than `$(method)` keeps the string quotes visible in +the output, making it unambiguous that the user passed a `String` value (and +making copy-paste typos easy to spot). + +### Replace a dimension-mismatch guard (or add one that is missing) + +```julia +# Before — no check; size mismatch surfaces deep inside a linear algebra call +mean_store = zeros(output_dim, n_test) # pre-allocated buffer + +# After — explicit guard before the computation +size(mean_store) == (output_dim, size(inputs, 2)) || throw(DimensionMismatch(""" +Pre-allocated mean_store has the wrong shape for this prediction. + +Expected: + size(mean_store) == (output_dim, n_test) + +Got: + size(mean_store) = $(size(mean_store)) + output_dim = $output_dim + n_test = $(size(inputs, 2)) +""")) +``` + +### Preserve the original exception when catching and re-throwing + +```julia +# Before — PosDefException or SingularException silently discarded +try + cov_chol = cholesky(feature_cov) +catch e + error("Feature covariance factorization failed.") +end + +# After — root cause preserved, matrix state shown +try + cov_chol = cholesky(feature_cov) +catch e + throw(ArgumentError(""" +Feature covariance factorization failed during prediction. + +Got: + size(feature_cov) = $(size(feature_cov)) + isposdef(feature_cov) = $(isposdef(feature_cov)) + +Caused by: $(sprint(showerror, e)) + +Suggestion: + The regularization parameter in RandomFeatureMethod may be too small. + Try increasing it so the feature covariance stays positive definite. +""")) +end +``` + +`sprint(showerror, e)` formats as `"LinearAlgebra.PosDefException: matrix is not +Hermitian; Cholesky factorization failed."` — far more informative than `string(e)`. + +### Rewrite broken `@warn` interpolation + +```julia +# Before — condition number never actually interpolated (template string, not interpolation) +@warn "The provided regularization is poorly conditioned: κ(reg) = cond(regularization). Imprecision or SingularException during inversion may occur." + +# After — actual value shown +κ = cond(regularization) +@warn "Regularization matrix is poorly conditioned (κ = $κ, threshold = 1e8). Inversion may lose precision or throw SingularException." + +# Before (with string concatenation) +@warn string("Regularization is not positive definite.", "\n Applying posdef correction.") + +# After +@warn "Regularization matrix is not positive definite — applying posdef correction." +``` + +### Add loop context to an error thrown inside a batch loop + +```julia +# Before — user sees "not positive definite" with no idea which batch failed +for i in 1:n_batches + try + cov_chol = cholesky(C_batch[i]) + catch e + error("Cholesky factorization failed") + end +end + +# After — guard before cholesky, expose batch index and diagnostic state +for i in 1:n_batches + isposdef(C_batch[i]) || throw(ArgumentError(""" +Feature covariance is not positive definite at training batch $i / $n_batches. + +Expected: + A positive-definite covariance matrix at every batch step. + +Got: + batch index = $i / $n_batches + size(C_batch[i]) = $(size(C_batch[i])) + minimum eigval ≈ $(minimum(eigvals(Symmetric(C_batch[i])))) + +Suggestion: + Ensemble collapse or under-regularisation can cause this near batch $i. + Consider increasing the regularization parameter in RandomFeatureMethod. +""")) + cov_chol = cholesky(C_batch[i]) +end +``` + +Key points: +- **Move the guard before the failing call** so the message fires with full batch + state still in scope. +- **Report the loop variable** (`i`) and its upper bound so the user knows whether + the failure is early (batch 2/200) or late (batch 198/200). +- **Include one diagnostic scalar** — the minimum eigenvalue rather than dumping + the full matrix. + +### Extract a duplicated error into a helper + +The "xi" parameter guard appears identically in both `ScalarFeature` and +`VectorFeature` constructors. Before extraction, byte-for-byte identical 8-line +blocks appear at both call sites: + +```julia +# Before — same block in both ScalarFeature and VectorFeature constructors + +# in ScalarFeature: +if "xi" ∉ get_name(get_parameter_distribution(feature_sampler)) + throw(ArgumentError( + " Named parameter \"xi\" not found in names of parameter_distribution. " * + " \n Please provide the name \"xi\" to the distribution used to sample the features", + )) +end + +# in VectorFeature: byte-for-byte identical block +if "xi" ∉ get_name(get_parameter_distribution(feature_sampler)) + throw(ArgumentError( + " Named parameter \"xi\" not found in names of parameter_distribution. " * + " \n Please provide the name \"xi\" to the distribution used to sample the features", + )) +end +``` + +After extraction, both constructors call one helper defined in `src/ErrorMessages.jl` +(promoted there because two different source files share it): + +```julia +# After — shared helper in src/ErrorMessages.jl + +## Error helpers + +@noinline function _throw_missing_xi(pd; where::Symbol) + throw(ArgumentError(""" +$where: parameter distribution must include a component named "xi" for feature sampling. + +Expected: + "xi" ∈ get_name(parameter_distribution) + +Got: + available names = $(get_name(pd)) + +Suggestion: + Add a ParameterDistribution component named "xi" when constructing FeatureSampler, + e.g. via constrained_gaussian("xi", μ, σ). +""")) +end + +# Both call sites collapse to a single readable guard line each: +# in ScalarFeature constructor: +"xi" ∈ get_name(get_parameter_distribution(feature_sampler)) || + _throw_missing_xi(get_parameter_distribution(feature_sampler); where = :ScalarFeature) + +# in VectorFeature constructor: +"xi" ∈ get_name(get_parameter_distribution(feature_sampler)) || + _throw_missing_xi(get_parameter_distribution(feature_sampler); where = :VectorFeature) +``` + +Key points: +- The `where::Symbol` kwarg embeds the calling type name so diagnostics stay + specific even though the body is shared. +- `@noinline` keeps the error path out of the hot constructor body. +- The helper lives in `src/ErrorMessages.jl` because two source files call it — + the rule for promoting out of a single file. + +### Helper with optional loop context + +The feature-parameter validation loop currently reports no position when a dict +is missing a required key — the user sees "missing sigma" with no idea which entry +in the array triggered it. Extracting into a helper adds the index and makes the +same helper reusable wherever that validation appears: + +```julia +# Before — inline block, no loop index in the message +for params in feature_param_list + if "sigma" ∉ keys(params) + throw(ArgumentError(""" +Feature parameter dict is missing required key "sigma". + +Expected keys: + "sigma" (required), plus any additional feature-specific parameters + +Got keys: + $(sort(collect(string.(keys(params))))) + +Suggestion: + Ensure each parameter dict includes "sigma" => . +""")) + end +end + +# After — helper with optional loop context at the bottom of the file + +@noinline function _throw_missing_sigma(got_keys; index = nothing, total = nothing) + loop_ctx = isnothing(index) ? "" : """ + +Loop context: + param index = $index (of $total)""" + throw(ArgumentError(""" +Feature parameter dict is missing required key "sigma".$loop_ctx + +Expected keys: + "sigma" (required), plus any additional feature-specific parameters + +Got keys: + $got_keys + +Suggestion: + Ensure each parameter dict includes "sigma" => . +""")) +end + +# Call site — loop now reports position: +for (i, params) in enumerate(feature_param_list) + "sigma" ∈ keys(params) || + _throw_missing_sigma( + sort(collect(string.(keys(params)))); index = i, total = length(feature_param_list), + ) +end + +# The same helper works outside a loop — omit the kwargs and the Loop context +# section is silently suppressed: +"sigma" ∈ keys(feature_parameters) || + _throw_missing_sigma(sort(collect(string.(keys(feature_parameters))))) +``` + +Key points: +- `index` and `total` default to `nothing`; the `Loop context:` section is + rendered only when they are provided. +- Switching `for params in ...` to `for (i, params) in enumerate(...)` is the + only loop-side change needed to expose the index. +- The user now knows *which* parameter dict failed, not just that one of them did. + +--- + +## Non-goals + +- Do not rewrite every low-level exception in the package. Focus on user-facing + API boundaries and sites explicitly identified. +- Do not suppress Julia stack traces. The goal is clearer diagnostics, not + silenced errors. +- Do not add verbosity for its own sake. A short, clear message beats a long, + generic one. +- Do not expose internal linear algebra variable names or dispatch details when + domain-level terminology exists. +- Do not extract truly short errors (≤3 lines) at a single call site — the + inline form is easier to grep and keeps cause and message co-located. A single + summary line, or a summary plus one Got line, is the ceiling for inline. From ede8dfe533088ca23fe34b873762696b1508c311 Mon Sep 17 00:00:00 2001 From: odunbar Date: Wed, 20 May 2026 21:05:53 -0700 Subject: [PATCH 2/5] apply show method, add changes, iterate --- .claude/skills/base-show/SKILL.md | 46 ++++++++++ src/RandomFeatures.jl | 1 + src/show.jl | 147 ++++++++++++++++++++++++++++++ test/runtests.jl | 2 +- test/show/runtests.jl | 127 ++++++++++++++++++++++++++ 5 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 src/show.jl create mode 100644 test/show/runtests.jl diff --git a/.claude/skills/base-show/SKILL.md b/.claude/skills/base-show/SKILL.md index 9bf6775..5d05d38 100644 --- a/.claude/skills/base-show/SKILL.md +++ b/.claude/skills/base-show/SKILL.md @@ -102,6 +102,9 @@ grep -nrE 'Base\.(show|summary)' src/ Skip any type that already has a custom `Base.show` or `Base.summary` method — do not overwrite existing customization. +Also skip **zero-field structs** — their default show (`Cosine()`, `Relu()`) is already +compact and informative; there is nothing to improve. + ### Step 3 — Write show and summary methods For each noisy type without existing methods, write **both** a `Base.show` and a @@ -173,6 +176,21 @@ default to `src/show.jl` if no prior convention exists. If creating `src/show.jl`, add `include("show.jl")` to the main module file after the type definitions it references. +**Multi-submodule packages**: When each `.jl` file opens its own `module` (e.g. +`module Utilities ... end`), the types live in separate namespaces. In this case, +include `show.jl` from the *parent* module file **after all submodule includes**, and +reference types by their submodule-qualified name: + +```julia +# In show.jl, included from module RandomFeatures after all submodule includes: +function Base.show(io::IO, ::MIME"text/plain", x::Utilities.Decomposition) ... +function Base.show(io::IO, x::Features.ScalarFeature) ... +``` + +Inside show methods placed at the parent-module level, **prefer direct field access** +(`x.n_features`) over calling submodule accessors (`Features.get_n_features(x)`) — +the accessor may not be in scope, and field access is just as clear. + ### Step 4 — Write unit tests Write one test block per type, covering `show` (full and compact), and `summary`. Each @@ -188,6 +206,34 @@ test block must: - For `summary`: capture output with `sprint(summary, instance)` and assert that it contains the type name and produces exactly one line (no `'\n'` in output). +**Test runner registration**: If the package has a top-level `test/runtests.jl` that +iterates over a list of submodule test directories, add `"show"` to that list so the +new tests run as part of `Pkg.test()`. + +**DRY helpers for multiple types**: When testing more than two types, extract the +repeating assertions into helpers to avoid noisy repetition: + +```julia +function check_full(x, typename) + out = sprint(show, MIME("text/plain"), x) + @test occursin(typename, out) + @test count(==('\n'), out) <= 10 +end +function check_compact(x, typename) + out2 = sprint(show, x) + @test occursin(typename, out2) && !occursin('\n', out2) + @test out2 == sprint(show, MIME("text/plain"), x; context = :compact => true) +end +function check_summary(x, typename) + out = sprint(summary, x) + @test occursin(typename, out) && !occursin('\n', out) +end +``` + +Then each type's test block becomes three lines: `check_full(x, "T")`, +`check_compact(x, "T")`, `check_summary(x, "T")`. Add any type-specific assertions +(e.g., that the feature count appears in the compact hint) after the helpers. + **Bespoke 2-arg shows (retrofit case):** Some types may already have a 2-arg show that intentionally does not include the type name or follow summary style — the method is doing something custom. Using a shared `check_compact(x, typename)` helper will diff --git a/src/RandomFeatures.jl b/src/RandomFeatures.jl index bfb91d8..2c3027a 100644 --- a/src/RandomFeatures.jl +++ b/src/RandomFeatures.jl @@ -20,5 +20,6 @@ include("Utilities.jl") # some additional tools include("Samplers.jl") # samples a distribution include("Features.jl") # builds a feature from the samples include("Methods.jl") # fits to data +include("show.jl") # Base.show / Base.summary for all public types end # module diff --git a/src/show.jl b/src/show.jl new file mode 100644 index 0000000..e5e81db --- /dev/null +++ b/src/show.jl @@ -0,0 +1,147 @@ +# ── Utilities / Decomposition ───────────────────────────────────────────────── + +function Base.show(io::IO, ::MIME"text/plain", x::Utilities.Decomposition) + if get(io, :compact, false) + show(io, x) + else + m, n = size(x.full_matrix) + println(io, "Decomposition") + println(io, " size : ", m, " × ", n) + println(io, " method: ", nameof(typeof(x.decomposition))) + end +end + +function Base.show(io::IO, x::Utilities.Decomposition) + m, n = size(x.full_matrix) + print(io, "Decomposition (", m, "×", n, ", ", nameof(typeof(x.decomposition)), ")") +end + +function Base.summary(io::IO, x::Utilities.Decomposition) + m, n = size(x.full_matrix) + print(io, "Decomposition (", m, "×", n, ", ", nameof(typeof(x.decomposition)), ")") +end + +# ── Samplers ────────────────────────────────────────────────────────────────── + +function Base.show(io::IO, ::MIME"text/plain", x::Samplers.Sampler) + if get(io, :compact, false) + show(io, x) + else + println(io, "Sampler") + println(io, " rng : ", nameof(typeof(x.rng))) + println(io, " dist: ", nameof(typeof(x.parameter_distribution))) + end +end + +function Base.show(io::IO, x::Samplers.Sampler) + print(io, "Sampler (", nameof(typeof(x.rng)), ")") +end + +function Base.summary(io::IO, x::Samplers.Sampler) + print(io, "Sampler (", nameof(typeof(x.rng)), ")") +end + +# ── ScalarFeatures ──────────────────────────────────────────────────────────── + +function Base.show(io::IO, ::MIME"text/plain", x::Features.ScalarFeature) + if get(io, :compact, false) + show(io, x) + else + n = x.n_features + println(io, "ScalarFeature") + println(io, " n_features : ", n) + println(io, " scalar_function: ", nameof(typeof(x.scalar_function))) + if !isnothing(x.feature_parameters) + np = length(x.feature_parameters) + println(io, " feature_params : Dict with ", np, " entr", np == 1 ? "y" : "ies") + end + end +end + +function Base.show(io::IO, x::Features.ScalarFeature) + print(io, "ScalarFeature (", x.n_features, " feature", + x.n_features == 1 ? "" : "s", ", ", nameof(typeof(x.scalar_function)), ")") +end + +function Base.summary(io::IO, x::Features.ScalarFeature) + print(io, "ScalarFeature (", x.n_features, " feature", + x.n_features == 1 ? "" : "s", ", ", nameof(typeof(x.scalar_function)), ")") +end + +# ── VectorFeatures ──────────────────────────────────────────────────────────── + +function Base.show(io::IO, ::MIME"text/plain", x::Features.VectorFeature) + if get(io, :compact, false) + show(io, x) + else + n = x.n_features + println(io, "VectorFeature") + println(io, " n_features : ", n) + println(io, " output_dim : ", x.output_dim) + println(io, " scalar_function: ", nameof(typeof(x.scalar_function))) + if !isnothing(x.feature_parameters) + np = length(x.feature_parameters) + println(io, " feature_params : Dict with ", np, " entr", np == 1 ? "y" : "ies") + end + end +end + +function Base.show(io::IO, x::Features.VectorFeature) + print(io, "VectorFeature (", x.n_features, " features → ", x.output_dim, "D, ", + nameof(typeof(x.scalar_function)), ")") +end + +function Base.summary(io::IO, x::Features.VectorFeature) + print(io, "VectorFeature (", x.n_features, " features → ", x.output_dim, "D, ", + nameof(typeof(x.scalar_function)), ")") +end + +# ── RandomFeatureMethod / Fit ───────────────────────────────────────────────── + +function Base.show(io::IO, ::MIME"text/plain", x::Methods.RandomFeatureMethod) + if get(io, :compact, false) + show(io, x) + else + bs = x.batch_sizes + println(io, "RandomFeatureMethod") + println(io, " feature type : ", nameof(typeof(x.random_feature))) + println(io, " n_features : ", x.random_feature.n_features) + println(io, " regularization: ", nameof(typeof(x.regularization))) + println(io, " batch_sizes : train=", bs["train"], + ", test=", bs["test"], ", feature=", bs["feature"]) + println(io, " threading : ", x.tullio_threading) + end +end + +function Base.show(io::IO, x::Methods.RandomFeatureMethod) + print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), + ", n=", x.random_feature.n_features, ")") +end + +function Base.summary(io::IO, x::Methods.RandomFeatureMethod) + print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), + ", n=", x.random_feature.n_features, ")") +end + +function Base.show(io::IO, ::MIME"text/plain", x::Methods.Fit) + if get(io, :compact, false) + show(io, x) + else + n = length(x.coeffs) + m, k = size(x.feature_factors.full_matrix) + println(io, "Fit") + println(io, " coeffs : ", n, " element", n == 1 ? "" : "s") + println(io, " feature_factors: Decomposition (", m, "×", k, ")") + println(io, " regularization : ", nameof(typeof(x.regularization))) + end +end + +function Base.show(io::IO, x::Methods.Fit) + n = length(x.coeffs) + print(io, "Fit (", n, " coeff", n == 1 ? "" : "s", ")") +end + +function Base.summary(io::IO, x::Methods.Fit) + n = length(x.coeffs) + print(io, "Fit (", n, " coeff", n == 1 ? "" : "s", ")") +end diff --git a/test/runtests.jl b/test/runtests.jl index 411a9ac..eb679c1 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -26,7 +26,7 @@ end end end - for submodule in ["Utilities", "Samplers", "Features", "Methods"] + for submodule in ["Utilities", "Samplers", "Features", "Methods", "show"] if all_tests || has_submodule(submodule) || "RandomFeatures" in ARGS include_test(submodule) end diff --git a/test/show/runtests.jl b/test/show/runtests.jl new file mode 100644 index 0000000..abed7c6 --- /dev/null +++ b/test/show/runtests.jl @@ -0,0 +1,127 @@ +using Test +using LinearAlgebra +using Random +using StableRNGs +using Distributions + +using RandomFeatures.Utilities +using RandomFeatures.Samplers +using RandomFeatures.Features +using RandomFeatures.Methods +using RandomFeatures.DataContainers +using RandomFeatures.ParameterDistributions + +seed = 9999 + +function check_full(x, typename) + out = sprint(show, MIME("text/plain"), x) + @test occursin(typename, out) + @test count(==('\n'), out) <= 10 +end + +function check_compact(x, typename) + out2 = sprint(show, x) + @test occursin(typename, out2) + @test !occursin('\n', out2) + out3 = sprint(show, MIME("text/plain"), x; context = :compact => true) + @test out2 == out3 +end + +function check_summary(x, typename) + out = sprint(summary, x) + @test occursin(typename, out) + @test !occursin('\n', out) +end + +@testset "show" begin + + rng = StableRNG(seed) + M = 8 + mat = Matrix(1.0I, M, M) + 0.1 * (rand(rng, M, M) + rand(rng, M, M)') + + @testset "Decomposition (svd)" begin + dc = Decomposition(mat, "svd") + check_full(dc, "Decomposition") + check_compact(dc, "Decomposition") + check_summary(dc, "Decomposition") + end + + @testset "Decomposition (cholesky)" begin + dc = Decomposition(mat, "cholesky") + check_full(dc, "Decomposition") + check_compact(dc, "Decomposition") + check_summary(dc, "Decomposition") + end + + @testset "Decomposition (pinv)" begin + dc = Decomposition(mat, "pinv") + check_full(dc, "Decomposition") + check_compact(dc, "Decomposition") + check_summary(dc, "Decomposition") + end + + μ_c = 0.0 + σ_c = 1.0 + pd = constrained_gaussian("xi", μ_c, σ_c, -Inf, Inf) + fsampler = FeatureSampler(pd, rng = copy(rng)) + + @testset "Sampler" begin + check_full(fsampler, "Sampler") + check_compact(fsampler, "Sampler") + check_summary(fsampler, "Sampler") + end + + n_features = 64 + + @testset "ScalarFeature (Cosine)" begin + sf = ScalarFourierFeature(n_features, FeatureSampler(pd, rng = copy(rng))) + check_full(sf, "ScalarFeature") + check_compact(sf, "ScalarFeature") + check_summary(sf, "ScalarFeature") + # hint contains feature count + @test occursin(string(n_features), sprint(show, sf)) + end + + @testset "ScalarFeature (Relu)" begin + sf = ScalarNeuronFeature(n_features, FeatureSampler(pd, rng = copy(rng))) + check_full(sf, "ScalarFeature") + check_compact(sf, "ScalarFeature") + check_summary(sf, "ScalarFeature") + end + + output_dim = 3 + pd_nd = constrained_gaussian("xi", μ_c, σ_c, -Inf, Inf, repeats = 4) + vfsampler = FeatureSampler(pd_nd, output_dim, rng = copy(rng)) + + @testset "VectorFeature" begin + vf = VectorFourierFeature(n_features, output_dim, vfsampler) + check_full(vf, "VectorFeature") + check_compact(vf, "VectorFeature") + check_summary(vf, "VectorFeature") + # hint contains output_dim + @test occursin(string(output_dim), sprint(show, vf)) + end + + @testset "RandomFeatureMethod" begin + sf = ScalarFourierFeature(n_features, FeatureSampler(pd, rng = copy(rng))) + rfm = RandomFeatureMethod(sf) + check_full(rfm, "RandomFeatureMethod") + check_compact(rfm, "RandomFeatureMethod") + check_summary(rfm, "RandomFeatureMethod") + @test occursin(string(n_features), sprint(show, rfm)) + end + + @testset "Fit" begin + sf = ScalarFourierFeature(n_features, FeatureSampler(pd, rng = copy(rng))) + rfm = RandomFeatureMethod(sf) + n_data = 30 + x = rand(rng, 1, n_data) + y = rand(rng, 1, n_data) + io_pairs = PairedDataContainer(x, y, data_are_columns = true) + f = fit(rfm, io_pairs) + check_full(f, "Fit") + check_compact(f, "Fit") + check_summary(f, "Fit") + end + +end From a79a49fc4eaf32581528ccbe527af37c88d94480 Mon Sep 17 00:00:00 2001 From: odunbar Date: Thu, 21 May 2026 00:47:17 -0700 Subject: [PATCH 3/5] applied docstrings skill and iterated --- .claude/skills/docstrings/SKILL.md | 80 ++++++++++++++++++++++++------ docs/src/API/Features.md | 46 +++++++++-------- docs/src/API/Methods.md | 39 ++++++++------- docs/src/API/Utilities.md | 1 + src/Features.jl | 3 ++ src/Methods.jl | 56 +++++++++++++++------ src/Samplers.jl | 22 +++++--- src/ScalarFeatures.jl | 27 ++++++++-- src/ScalarFunctions.jl | 70 ++++++++++++++++++++++++-- src/Utilities.jl | 31 ++++++++++-- src/VectorFeatures.jl | 28 ++++++++--- 11 files changed, 310 insertions(+), 93 deletions(-) diff --git a/.claude/skills/docstrings/SKILL.md b/.claude/skills/docstrings/SKILL.md index 2e82b8e..1bd6ed4 100644 --- a/.claude/skills/docstrings/SKILL.md +++ b/.claude/skills/docstrings/SKILL.md @@ -11,9 +11,10 @@ description: > or asks to document a type or function, sync the API pages, or keep the API index up to date. Also use it when the user asks to "write docs for" or "add docs to" source files, or when a CI failure mentions missing or incomplete - docstrings. In RandomFeatures.jl, priority targets are: scalar function types - (Cosine, Relu, Gelu, etc. have minimal docstrings), get_output_dim (missing), - and any function whose docstring is a bare $(TYPEDSIGNATURES) with no prose. + docstrings. In RandomFeatures.jl, the 2026-05-20 audit resolved all bare stubs; + common failure modes to watch for are inaccurate return-type claims, getter prose + copy-pasted across dispatches, and jldoctests using `using PackageName` for symbols + that live only in a submodule and need `using PackageName.SubModule`. --- # docstrings @@ -77,6 +78,24 @@ the definition. Produce a prioritised list: 4. **Incomplete** — prose present but key sections absent (missing `# Arguments`, `# Examples`, or field strings not describing semantic role). +**Accuracy checks on existing prose.** Alongside the coverage gaps above, run two +quick passes so correctness bugs in *existing* docstrings don't survive the edit cycle: + +- **Return-type claims.** For any function whose existing docstring prose contains + "Returns a " or "Returns an ", read the actual `return` statement(s) in the source + and verify the claimed shape matches. A common failure mode is a one-liner wrapper + whose docstring says "Returns an `output_dim × n_samples` array" but the + implementation returns a `(array, features)` tuple. Flag these as category 4 + (Incomplete) and correct the prose in Step 3. + +- **Getter semantic claims.** For getter functions (names beginning with `get_`) + that assert a specific value or type in prose — e.g. "equals 1", "returns a Bool", + "always positive" — verify the claim against the actual implementation body for + *every* dispatch of that getter name. Copy-paste errors across overloads are common: + a `get_output_dim` defined on a vector type may have inherited prose that says + "equals 1 for scalar-valued features". Flag these as category 4 and correct the + prose so each overload's docstring only makes claims that hold for its own dispatch. + **Also scan every function in the file for old-style docstrings**, regardless of whether it is exported. An old-style docstring is one that uses an indented function-name header (e.g. ` my_func(arg1, arg2)`) and/or an `Args:` / @@ -108,6 +127,14 @@ convert it to the new format: list, remove the manual list. - Preserve any genuine prose that was in the old `# Constructor` section if it explains non-obvious behaviour; discard boilerplate signature repetition. +- After adding `$(METHODLIST)`, check whether any exported factory function + builds this struct under a different name (e.g. `fit` for `Fit`, + `FeatureSampler` for `Sampler`). If so, insert a prose note naming that + factory in the `# Constructors` section, immediately before `$(METHODLIST)`, + following the **Named constructors** pattern below. Do *not* bury the factory + reference only in the opening prose — putting it inside `# Constructors` is + what makes it discoverable to readers scanning for "how do I build one of + these?". #### Named constructors (factory functions) @@ -179,6 +206,20 @@ one — and leave the rest undocumented. Internal framework methods that are exported for extensibility but not called directly by end users do not need documentation even if exported. +**Overloads with different return types are separate concepts.** The one-docstring +rule is for type-parameter specialisations that do the same thing for different +argument types. It does *not* apply when two overloads of the same name return +fundamentally different shapes — e.g. a 3-argument form that returns a +`(result, features)` tuple while a 4-argument form returns only `result`. These +are distinct contracts and both deserve a docstring, each with a clear description +of what it returns. + +Exception: if both variants are only ever called through a single, higher-level +public function (e.g. `predict` wraps both `predictive_mean` and `predictive_cov` +internally), leave the implementation variants undocumented and document only the +public entry point. The test is whether a user would ever plausibly call the variant +directly; if not, it does not need a docstring. + #### Old-style function docstrings (all functions, not just exported) Convert any docstring that uses an indented function-name header or an `Args:` / @@ -370,6 +411,12 @@ expect. Apply them consistently. other dispatch methods remain undocumented. Do **not** add `$(METHODLIST)` to function docstrings — `$(TYPEDSIGNATURES)` already surfaces all overloads. `$(METHODLIST)` belongs only in struct docstrings (inside `# Constructors`). + **Exception — convenience overloads with distinct kwargs**: If a secondary overload + introduces keyword arguments or a shorthand interface not visible in the primary + overload's signature (e.g. an `output_dim::Int` convenience argument or a + `uniform_shift_bounds` keyword), do not silently delete that information. Instead, + remove the secondary overload's docstring and fold a brief `# Convenience overloads` + note into the primary docstring, naming the extra kwargs and when to prefer them. - **`# Arguments` section**: add after the opening prose for any function with more than two parameters, or where argument semantics are non-obvious. Format: `` - `name`: description [unit] ``. @@ -380,9 +427,14 @@ expect. Apply them consistently. a blank line. Documenter.jl rejects blocks where two prompts appear consecutively without an intervening blank line. If a statement produces no output, end it with a semicolon and add a blank line before the next prompt. -- If the doctest references any name from the package, the first statement must - be `julia> using RandomFeatures` (followed by a blank line). Do not assume the - package is already in scope. +- **jldoctest import line**: Before writing any `# Examples` block, verify that + the symbol is actually accessible via `using PackageName` at the top level. Read + the main module file (`src/PackageName.jl`) and check whether the symbol appears + in a top-level `export` statement or is brought in via `using .SubModule`. If + neither is true, the symbol lives only in the submodule — use the full path as + the import line: `julia> using PackageName.SubModule`. Using the wrong import + causes a silent doctest failure (`UndefVarError`) even though the symbol is + genuinely public. Do not assume the package is already in scope. ## Quality criteria @@ -422,23 +474,23 @@ struct Fit{V <: AbstractVector, USorM <: Union{UniformScaling, AbstractMatrix}} ... end -## After — NEW format: prose only, $(TYPEDEF) for signature, $(METHODLIST) for constructors +## After — NEW format: $(TYPEDEF), prose, $(TYPEDFIELDS), and factory method in # Constructors +## (factory `fit` has a different name from the struct, so $(METHODLIST) adds nothing; +## the # Constructors section is pure prose naming the factory instead) """ -Hold the coefficients and matrix decomposition that describe a set of fitted random features. - -Instances are produced by `fit(rfm, inputs, outputs)` and passed to `predict` and -`predictive_mean` / `predictive_cov` to generate predictions at new input locations. - $(TYPEDEF) -# Fields +Holds the coefficients and matrix decomposition produced by `fit`, describing a trained random +feature regression model. Pass to `predict`, `predictive_mean`, or `predictive_cov` to obtain +predictions at new input locations. $(TYPEDFIELDS) # Constructors -$(METHODLIST) +Produced by `fit(rfm, input_output_pairs; decomposition_type = "cholesky")` — not intended to +be constructed directly. """ struct Fit{V <: AbstractVector, USorM <: Union{UniformScaling, AbstractMatrix}} ... diff --git a/docs/src/API/Features.md b/docs/src/API/Features.md index e9503e7..8a7e4f9 100644 --- a/docs/src/API/Features.md +++ b/docs/src/API/Features.md @@ -4,6 +4,14 @@ CurrentModule = RandomFeatures.Features ``` +## Abstract types + +```@docs +RandomFeature +``` + +## Getter functions + ```@docs get_scalar_function get_feature_sampler @@ -14,42 +22,40 @@ get_output_dim sample(rf::RandomFeature) ``` -# [Scalar Features](@id scalar-features) +## [Scalar Features](@id scalar-features) ```@docs ScalarFeature ScalarFourierFeature ScalarNeuronFeature -build_features(rf::ScalarFeature,inputs::AbstractMatrix,atch_feature_idx::AbstractVector{Int}) +build_features(rf::ScalarFeature, inputs::AbstractMatrix, batch_feature_idx::AbstractVector) ``` -# [Vector Features](@id vector-features) + +## [Vector Features](@id vector-features) ```@docs VectorFeature VectorFourierFeature VectorNeuronFeature -build_features(rf::VectorFeature,inputs::AbstractMatrix,atch_feature_idx::AbstractVector{Int}) +build_features(rf::VectorFeature, inputs::AbstractMatrix, batch_feature_idx::AbstractVector) ``` -# [Scalar Functions](@id scalar-functions) +## [Scalar Functions](@id scalar-functions) ```@docs ScalarFunction ScalarActivation apply_scalar_function -``` - -```@docs - Cosine - Relu - Lrelu - Gelu - Elu - Selu - Heaviside - SmoothHeaviside - Sawtooth - Softplus - Tansig - Sigmoid +Cosine +Relu +Lrelu +Gelu +Elu +Selu +Heaviside +SmoothHeaviside +Sawtooth +Softplus +Tansig +Sigmoid ``` \ No newline at end of file diff --git a/docs/src/API/Methods.md b/docs/src/API/Methods.md index c195e9f..9a51831 100644 --- a/docs/src/API/Methods.md +++ b/docs/src/API/Methods.md @@ -5,23 +5,24 @@ CurrentModule = RandomFeatures.Methods ``` ```@docs - RandomFeatureMethod - Fit - get_random_feature - get_batch_sizes - get_batch_size - get_regularization - sample(rfm::RandomFeatureMethod) - get_feature_factors - get_coeffs - fit - predict - predict! - predictive_mean - predictive_mean! - predictive_cov - predictive_cov! - predict_prior - predict_prior_mean - predict_prior_cov +RandomFeatureMethod +Fit +get_random_feature +get_batch_sizes +get_batch_size +get_regularization +get_tullio_threading +sample(rfm::RandomFeatureMethod) +get_feature_factors +get_coeffs +fit +predict +predict! +predictive_mean +predictive_mean! +predictive_cov +predictive_cov! +predict_prior +predict_prior_mean +predict_prior_cov ``` diff --git a/docs/src/API/Utilities.md b/docs/src/API/Utilities.md index dd157fd..a496a0f 100644 --- a/docs/src/API/Utilities.md +++ b/docs/src/API/Utilities.md @@ -17,6 +17,7 @@ StoredInvType Factor PseInv get_decomposition +get_inv_decomposition get_full_matrix get_parametric_type linear_solve diff --git a/src/Features.jl b/src/Features.jl index 2566503..d6efb4d 100644 --- a/src/Features.jl +++ b/src/Features.jl @@ -6,6 +6,9 @@ import StatsBase: sample using EnsembleKalmanProcesses.ParameterDistributions, DocStringExtensions, RandomFeatures.Samplers, Tullio, LoopVectorization +""" +Abstract supertype for all random feature approximation models mapping inputs to random feature embeddings. +""" abstract type RandomFeature end include("ScalarFeatures.jl") include("VectorFeatures.jl") diff --git a/src/Methods.jl b/src/Methods.jl index 01268b1..df697f3 100644 --- a/src/Methods.jl +++ b/src/Methods.jl @@ -34,9 +34,15 @@ export RandomFeatureMethod, """ $(TYPEDEF) -Holds configuration for the random feature fit +Holds the configuration for a random feature regression model, including the feature object, +batch sizes for training and prediction, and the regularisation matrix. $(TYPEDFIELDS) + +# Constructors + +`RandomFeatureMethod(random_feature; regularization, batch_sizes, tullio_threading, regularization_inverted)` — +see the constructor docstring for defaults and validation behaviour. """ struct RandomFeatureMethod{S <: AbstractString, USorM <: Union{UniformScaling, AbstractMatrix}} "The random feature object" @@ -144,9 +150,16 @@ get_batch_size(rfm::RandomFeatureMethod, key::S) where {S <: AbstractString} = g """ $(TYPEDEF) -Holds the coefficients and matrix decomposition that describe a set of fitted random features. +Holds the coefficients and matrix decomposition produced by `fit`, describing a trained random +feature regression model. Pass to `predict`, `predictive_mean`, or `predictive_cov` to obtain +predictions at new input locations. $(TYPEDFIELDS) + +# Constructors + +Produced by `fit(rfm, input_output_pairs; decomposition_type = "cholesky")` — not intended to +be constructed directly. """ struct Fit{V <: AbstractVector, USorM <: Union{UniformScaling, AbstractMatrix}} "The `LinearAlgreba` matrix decomposition of `(1 / m) * Feature^T * regularization^-1 * Feature + I`" @@ -247,9 +260,12 @@ function fit( end """ - $(TYPEDSIGNATURES) - - Makes a prediction of mean and (co)variance of fitted features on new input data +$(TYPEDSIGNATURES) + +Return the predictive mean and (co)variance of the fitted random feature model evaluated at `new_inputs`. + +Returns a `(mean, cov)` tuple where `mean` is an `output_dim × n_samples` array and `cov` is +an `output_dim × output_dim × n_samples` array. """ function predict(rfm::RandomFeatureMethod, fit::Fit, new_inputs::DataContainer; kwargs...) pred_mean, features = predictive_mean(rfm, fit, new_inputs; kwargs...) @@ -358,8 +374,11 @@ end """ $(TYPEDSIGNATURES) -Makes a prediction of mean of fitted features on new input data. -Returns a `output_dim` x `n_samples` array. +Return the predictive mean of the fitted random feature model evaluated at `new_inputs`. + +Returns a tuple `(mean, features)` where `mean` is an `output_dim × n_samples` array and +`features` is the `n_samples × output_dim × n_features` feature matrix, which can be passed +directly to `predictive_cov` to avoid rebuilding features. """ predictive_mean(rfm::RandomFeatureMethod, fit::Fit, new_inputs::DataContainer; kwargs...) = predictive_mean(rfm, get_coeffs(fit), new_inputs; kwargs...) @@ -474,10 +493,12 @@ function predictive_mean!( end """ - $(TYPEDSIGNATURES) - - Makes a prediction of (co)variance of fitted features on new input data. -Returns a `output_dim` x `output_dim` x `n_samples` array +$(TYPEDSIGNATURES) + +Return the predictive covariance of the fitted random feature model evaluated at `new_inputs`, +using `prebuilt_features` to avoid rebuilding the feature matrix. + +Returns an `output_dim × output_dim × n_samples` array. """ function predictive_cov( rfm::RandomFeatureMethod, @@ -509,10 +530,15 @@ function predictive_cov(rfm::RandomFeatureMethod, fit::Fit, new_inputs::DataCont end """ - $(TYPEDSIGNATURES) - - Makes a prediction of (co)variance of fitted features on new input data. -Writes into a provided `output_dim` x `output_dim` x `n_samples` array: `cov_store`, and uses provided `n_samples` x `output_dim` x `n_features` buffer. +$(TYPEDSIGNATURES) + +Compute the predictive covariance of the fitted random feature model at `new_inputs`, writing into +pre-allocated arrays and using `prebuilt_features` to avoid rebuilding the feature matrix. + +# Arguments +- `cov_store`: pre-allocated `output_dim × output_dim × n_samples` array for the covariance output. +- `buffer`: pre-allocated `n_samples × output_dim × n_features` working array. +- `prebuilt_features`: `n_samples × output_dim × n_features` feature matrix. """ function predictive_cov!( rfm::RandomFeatureMethod, diff --git a/src/Samplers.jl b/src/Samplers.jl index 26fee70..feb0553 100644 --- a/src/Samplers.jl +++ b/src/Samplers.jl @@ -9,9 +9,16 @@ export Sampler, FeatureSampler, get_parameter_distribution, get_rng, sample """ $(TYPEDEF) -Wraps the parameter distributions used to sample random features +Wraps the parameter distributions and random number generator used to draw random feature parameters. $(TYPEDFIELDS) + +# Constructors + +- `FeatureSampler(parameter_distribution, bias_distribution; rng)` — explicit `ParameterDistribution` + bias, or `nothing` for no bias term. +- `FeatureSampler(parameter_distribution, output_dim; uniform_shift_bounds, rng)` — uniform bias over + `output_dim` dimensions; bounds default to `[0, 2π]`. """ struct Sampler{RNG <: AbstractRNG} "A probability distribution, possibly with constraints" @@ -23,7 +30,13 @@ end """ $(TYPEDSIGNATURES) -basic constructor for a `Sampler` +Construct a `Sampler` from a `parameter_distribution` and an optional `bias_distribution`. + +When `bias_distribution` is `nothing`, no bias term is added to the feature inner product. +When a `ParameterDistribution` is supplied, it is combined with `parameter_distribution` +into a single joint distribution before wrapping in the `Sampler`. + +Pass a `ParameterDistribution` with name `"xi"` for the feature weights. """ function FeatureSampler( parameter_distribution::ParameterDistribution, @@ -38,11 +51,6 @@ function FeatureSampler( end end -""" -$(TYPEDSIGNATURES) - -one can conveniently specify the bias as a uniform-shift `uniform_shift_bounds` with `output_dim` dimensions -""" function FeatureSampler( parameter_distribution::ParameterDistribution, output_dim::Int; diff --git a/src/ScalarFeatures.jl b/src/ScalarFeatures.jl index ac6f396..0b1b20e 100644 --- a/src/ScalarFeatures.jl +++ b/src/ScalarFeatures.jl @@ -4,9 +4,14 @@ export build_features """ $(TYPEDEF) -Contains information to build and sample RandomFeatures mapping from N-D -> 1-D +Random feature model that maps N-dimensional inputs to 1-dimensional scalar outputs. $(TYPEDFIELDS) + +# Constructors + +- `ScalarFourierFeature(n_features, sampler; feature_parameters)` — cosine (Fourier) features. +- `ScalarNeuronFeature(n_features, sampler; activation_fun = Relu())` — activation-function features. """ struct ScalarFeature{S <: AbstractString, SF <: ScalarFunction} <: RandomFeature "Number of features" @@ -25,7 +30,11 @@ end """ $(TYPEDSIGNATURES) -basic constructor for a `ScalarFeature' +Construct a `ScalarFeature` with `n_features` random features sampled from `feature_sampler`, +applying `scalar_fun` to compute feature values. + +The `feature_sampler` parameter distribution must contain a named component `"xi"`. +`feature_parameters` must include the key `"sigma"` (output scale; default `1`). """ function ScalarFeature( n_features::Int, @@ -56,7 +65,10 @@ end """ $(TYPEDSIGNATURES) -Constructor for a `ScalarFeature` with cosine features +Construct a `ScalarFeature` with cosine (Fourier) features. + +The `sigma` feature parameter (default `sqrt(2)`) scales the output; use larger values for +wider kernel approximations. """ function ScalarFourierFeature( n_features::Int, @@ -69,7 +81,9 @@ end """ $(TYPEDSIGNATURES) -Constructor for a `ScalarFeature` with activation-function features (default ReLU) +Construct a `ScalarFeature` with a neural-network activation function (default `Relu`). + +Pass any `ScalarActivation` subtype as `activation_fun` to use a different activation. """ function ScalarNeuronFeature( n_features::Int, @@ -83,7 +97,10 @@ end """ $(TYPEDSIGNATURES) -builds features (possibly batched) from an input matrix of size (input dimension, number of samples) output of dimension (number of samples, 1, number features) +Build random features from an `input_dim × n_samples` input matrix, evaluating only the feature +indices in `batch_feature_idx`. + +Returns an `n_samples × 1 × length(batch_feature_idx)` array. """ function build_features( rf::ScalarFeature, diff --git a/src/ScalarFunctions.jl b/src/ScalarFunctions.jl index ea85f74..4e1bbd7 100644 --- a/src/ScalarFunctions.jl +++ b/src/ScalarFunctions.jl @@ -27,20 +27,33 @@ export apply_scalar_function """ $(TYPEDEF) -Type of a function mapping 1D -> 1D +Abstract supertype for all scalar functions mapping a single real value to a single real value. """ abstract type ScalarFunction end """ $(TYPEDSIGNATURES) -apply the scalar function `sf` pointwise to vectors or matrices +Apply the scalar function `sf` pointwise to each element of `r`. + +# Examples +```jldoctest +julia> using RandomFeatures.Features + +julia> apply_scalar_function(Relu(), [-1.0, 0.0, 1.0]) +3-element Vector{Float64}: + 0.0 + 0.0 + 1.0 +``` """ apply_scalar_function(sf::SF, r::A) where {SF <: ScalarFunction, A <: AbstractArray} = apply_scalar_function.(Ref(sf), r) # Ref(sf) treats sf as a scalar for the broadcasting """ $(TYPEDEF) + +Cosine scalar function: applies `cos(r)` pointwise to each input. """ struct Cosine <: ScalarFunction end function apply_scalar_function(sf::Cosine, r::FT) where {FT <: AbstractFloat} @@ -51,12 +64,14 @@ end """ $(TYPEDEF) -Type of scalar activation functions +Abstract supertype for neural-network activation functions (a subtype of `ScalarFunction`). """ abstract type ScalarActivation <: ScalarFunction end """ $(TYPEDEF) + +Rectified linear unit (ReLU) activation: returns `max(0, r)` for each input. """ struct Relu <: ScalarActivation end function apply_scalar_function(sa::Relu, r::FT) where {FT <: AbstractFloat} @@ -65,6 +80,8 @@ end """ $(TYPEDEF) + +Gaussian error linear unit (GELU) activation: `r * Φ(r)`, where `Φ` is the standard normal CDF. """ struct Gelu <: ScalarActivation end function apply_scalar_function(sa::Gelu, r::FT) where {FT <: AbstractFloat} @@ -74,6 +91,8 @@ end """ $(TYPEDEF) + +Heaviside step function: returns `0` for `r < 0`, `0.5` at `r == 0`, and `1` for `r > 0`. """ struct Heaviside <: ScalarActivation end function heaviside(x, y) @@ -92,6 +111,8 @@ end """ $(TYPEDEF) + +Sawtooth (triangle wave) activation: `max(0, min(2r, 2 - 2r))`. """ struct Sawtooth <: ScalarActivation end function apply_scalar_function(sa::Sawtooth, r::FT) where {FT <: AbstractFloat} @@ -100,6 +121,8 @@ end """ $(TYPEDEF) + +Softplus activation: numerically stable smooth approximation to ReLU, computed as `log(1 + exp(-|r|)) + max(r, 0)`. """ struct Softplus <: ScalarActivation end function apply_scalar_function(sa::Softplus, r::FT) where {FT <: AbstractFloat} @@ -108,6 +131,8 @@ end """ $(TYPEDEF) + +Hyperbolic tangent sigmoid activation: returns `tanh(r)`. """ struct Tansig <: ScalarActivation end function apply_scalar_function(sa::Tansig, r::FT) where {FT <: AbstractFloat} @@ -116,6 +141,8 @@ end """ $(TYPEDEF) + +Logistic sigmoid activation: returns `1 / (1 + exp(-r))`. """ struct Sigmoid <: ScalarActivation end function apply_scalar_function(sa::Sigmoid, r::FT) where {FT <: AbstractFloat} @@ -124,8 +151,17 @@ end """ $(TYPEDEF) + +Exponential linear unit (ELU) activation: linear for `r > 0`, scaled exponential `alpha * (exp(r) - 1)` for `r ≤ 0`. + +$(TYPEDFIELDS) + +# Constructors + +`Elu(; alpha = 1.0)` """ @kwdef struct Elu{FT <: AbstractFloat} <: ScalarActivation + "Scale applied to the negative exponential branch [dimensionless]; default `1.0`." alpha::FT = 1.0 end function apply_scalar_function(sa::Elu, r::FT) where {FT <: AbstractFloat} @@ -134,8 +170,17 @@ end """ $(TYPEDEF) + +Leaky ReLU activation: linear for `r > 0`, scaled by `alpha` for `r ≤ 0`. + +$(TYPEDFIELDS) + +# Constructors + +`Lrelu(; alpha = 0.01)` """ @kwdef struct Lrelu{FT <: AbstractFloat} <: ScalarActivation + "Slope of the negative linear branch [dimensionless]; default `0.01`." alpha::FT = 0.01 end function apply_scalar_function(sa::Lrelu, r::FT) where {FT <: AbstractFloat} @@ -144,9 +189,19 @@ end """ $(TYPEDEF) + +Scaled ELU (SELU) activation: self-normalising variant of ELU with fixed default scale parameters. + +$(TYPEDFIELDS) + +# Constructors + +`Selu(; alpha = 1.67326, lambda = 1.0507)` """ @kwdef struct Selu{FT <: AbstractFloat} <: ScalarActivation + "Negative-branch scale factor for self-normalisation [dimensionless]; default `1.67326`." alpha::FT = 1.67326 + "Global scale factor for self-normalisation [dimensionless]; default `1.0507`." lambda::FT = 1.0507 end function apply_scalar_function(sa::Selu, r::FT) where {FT <: AbstractFloat} @@ -155,8 +210,17 @@ end """ $(TYPEDEF) + +Smooth differentiable approximation to the Heaviside step function: `0.5 + atan(r / epsilon) / π`. + +$(TYPEDFIELDS) + +# Constructors + +`SmoothHeaviside(; epsilon = 0.01)` """ @kwdef struct SmoothHeaviside{FT <: AbstractFloat} <: ScalarActivation + "Width of the transition region [dimensionless]; smaller values approach the hard step function. Default `0.01`." epsilon::FT = 0.01 end function apply_scalar_function(sa::SmoothHeaviside, r::FT) where {FT <: AbstractFloat} diff --git a/src/Utilities.jl b/src/Utilities.jl index b5f4857..47378b3 100644 --- a/src/Utilities.jl +++ b/src/Utilities.jl @@ -41,9 +41,23 @@ end # Decomposition/linear solves for feature matrices """ - posdef_correct(mat::AbstractMatrix; tol::Real=1e8*eps()) +$(TYPEDSIGNATURES) + +Make a square matrix positive definite by symmetrising it and bounding the minimum eigenvalue below by `tol`. + +# Arguments +- `mat`: the square matrix to correct. +- `tol`: minimum eigenvalue tolerance; eigenvalues smaller than `tol` are shifted up [dimensionless]; default `1e12 * eps()`. + +# Examples +```jldoctest +julia> using LinearAlgebra, RandomFeatures.Utilities + +julia> M = [1.0 2.0; 2.0 1.0]; -Makes square matrix `mat` positive definite, by symmetrizing and bounding the minimum eigenvalue below by `tol` +julia> isposdef(posdef_correct(M)) +true +``` """ function posdef_correct(mat::AbstractMatrix; tol::Real = 1e12 * eps()) mat = deepcopy(mat) @@ -75,21 +89,30 @@ abstract type StoredInvType end """ $(TYPEDEF) + +Factorisation-based stored inverse type (SVD or Cholesky). """ abstract type Factor <: StoredInvType end """ $(TYPEDEF) + +Pseudoinverse-based stored inverse type. """ abstract type PseInv <: StoredInvType end """ $(TYPEDEF) -Stores a matrix along with a decomposition `T=Factor`, or pseudoinverse `T=PseInv`, and also computes the inverse of the Factored matrix (for several predictions this is actually the most computationally efficient action) - +Stores a matrix along with a decomposition `T=Factor`, or pseudoinverse `T=PseInv`, and also +computes the inverse of the factored matrix for efficient repeated linear solves. $(TYPEDFIELDS) + +# Constructors + +`Decomposition(mat, method; nugget = 1e12 * eps())` where `method` is one of `"svd"`, +`"cholesky"`, or `"pinv"`. """ struct Decomposition{T, M <: AbstractMatrix, MorF <: Union{AbstractMatrix, Factorization}} "The original matrix" diff --git a/src/VectorFeatures.jl b/src/VectorFeatures.jl index c005138..bc7ea28 100644 --- a/src/VectorFeatures.jl +++ b/src/VectorFeatures.jl @@ -4,9 +4,14 @@ export build_features """ $(TYPEDEF) -Contains information to build and sample RandomFeatures mapping from N-D -> M-D +Random feature model that maps N-dimensional inputs to M-dimensional vector outputs. $(TYPEDFIELDS) + +# Constructors + +- `VectorFourierFeature(n_features, output_dim, sampler; feature_parameters)` — cosine (Fourier) features. +- `VectorNeuronFeature(n_features, output_dim, sampler; activation_fun = Relu())` — activation-function features. """ struct VectorFeature{S <: AbstractString, SF <: ScalarFunction} <: RandomFeature "Number of features" @@ -27,7 +32,7 @@ end """ $(TYPEDSIGNATURES) -gets the output dimension (equals 1 for scalar-valued features) +Return the output dimension of `rf`. """ get_output_dim(rf::VectorFeature) = rf.output_dim @@ -36,7 +41,11 @@ get_output_dim(rf::VectorFeature) = rf.output_dim """ $(TYPEDSIGNATURES) -basic constructor for a `VectorFeature' +Construct a `VectorFeature` with `n_features` random features, `output_dim`-dimensional outputs, +sampled from `feature_sampler`, applying `scalar_fun` to compute feature values. + +The `feature_sampler` parameter distribution must contain a named component `"xi"`. +`feature_parameters` must include the key `"sigma"` (output scale; default `1`). """ function VectorFeature( n_features::Int, @@ -69,7 +78,9 @@ end """ $(TYPEDSIGNATURES) -Constructor for a `VectorFeature` with cosine features +Construct a `VectorFeature` with cosine (Fourier) features mapping to `output_dim`-dimensional outputs. + +The `sigma` feature parameter (default `sqrt(2)`) scales the output. """ function VectorFourierFeature( n_features::Int, @@ -83,7 +94,9 @@ end """ $(TYPEDSIGNATURES) -Constructor for a `VectorFeature` with activation-function features (default ReLU) +Construct a `VectorFeature` with a neural-network activation function (default `Relu`) mapping to `output_dim`-dimensional outputs. + +Pass any `ScalarActivation` subtype as `activation_fun` to use a different activation. """ function VectorNeuronFeature( n_features::Int, @@ -98,7 +111,10 @@ end """ $(TYPEDSIGNATURES) -builds features (possibly batched) from an input matrix of size (input dimension,number of samples) output of dimension (number of samples, output dimension, number features) +Build random features from an `input_dim × n_samples` input matrix, evaluating only the feature +indices in `batch_feature_idx`. + +Returns an `n_samples × output_dim × length(batch_feature_idx)` array. """ function build_features( rf::VectorFeature, From c21fada855032828d681f7eee0857832f775a83d Mon Sep 17 00:00:00 2001 From: odunbar Date: Thu, 21 May 2026 17:34:40 -0700 Subject: [PATCH 4/5] applied and iterated error-message-manager --- .claude/skills/error-message-manager/SKILL.md | 26 +++++++++++++++++++ src/ErrorMessages.jl | 17 ++++++++++++ src/Features.jl | 1 + src/Methods.jl | 26 +++++++++++++++---- src/ScalarFeatures.jl | 10 ++----- src/Utilities.jl | 21 ++++++++++----- src/VectorFeatures.jl | 10 ++----- test/Features/runtests.jl | 21 +++++++-------- test/Methods/runtests.jl | 5 +++- test/Utilities/runtests.jl | 5 +++- 10 files changed, 102 insertions(+), 40 deletions(-) create mode 100644 src/ErrorMessages.jl diff --git a/.claude/skills/error-message-manager/SKILL.md b/.claude/skills/error-message-manager/SKILL.md index 7a0841e..46eb3f1 100644 --- a/.claude/skills/error-message-manager/SKILL.md +++ b/.claude/skills/error-message-manager/SKILL.md @@ -220,6 +220,15 @@ which file to use by reading the top-level module file (e.g. `src/PackageName.jl for its `include(...)` list — then add `include("ErrorMessages.jl")` as the first `include` so every subsequent file sees the helpers without any `using`/`import`. +**Submodule scope trap**: When a shared helper lives in `src/ErrorMessages.jl`, +include it *inside every submodule that uses it*, not just in the parent module. +A function defined via `include("ErrorMessages.jl")` in `module RandomFeatures` +is invisible to `module Features` (a nested submodule). The fix: place +`include("ErrorMessages.jl")` at the top of the submodule's own file (e.g. +`Features.jl`), before the files that call it. The package will still load cleanly +with the helper in the wrong scope — only the `@test_throws` tests reveal the +mistake, making this easy to miss in review. + **Naming convention** ``` @@ -384,6 +393,23 @@ Three outcomes: | `@test_throws ` already present | Update the existing line to the new type | | No coverage at all | Add a new test | +**Check message content, not just type**: In Julia 1.8+, `@test_throws` returns +the caught exception, so you can pin key diagnostic text in the same block: + +```julia +let thrown = @test_throws ArgumentError f(bad_input) + @test contains(thrown.value.msg, "available names") # Got section present + @test contains(thrown.value.msg, repr(bad_input)) # received value interpolated +end +``` + +Add at least one `contains` check per new error site. Without this, a refactor can +preserve the exception type while silently dropping the Got section or the +interpolated value, and no test will catch it. Check for: a phrase from the summary +line, the Got section label (e.g. `"available names"`, `"missing required keys"`), +and `repr(bad_value)` when the message uses `repr`. The `let` block is the cleanest +form — it keeps the type assertion and the content assertions co-located. + For every site that needs a new test, add it in the matching `test//runtests.jl`: ```julia diff --git a/src/ErrorMessages.jl b/src/ErrorMessages.jl new file mode 100644 index 0000000..eb7bd73 --- /dev/null +++ b/src/ErrorMessages.jl @@ -0,0 +1,17 @@ +## Error helpers shared across multiple source files + +@noinline function _throw_missing_xi(pd; where::Symbol) + throw(ArgumentError(""" +$where: parameter distribution must include a component named "xi" for feature sampling. + +Expected: + "xi" ∈ get_name(parameter_distribution) + +Got: + available names = $(get_name(pd)) + +Suggestion: + Add a ParameterDistribution component named "xi" when constructing FeatureSampler, + e.g. via constrained_gaussian("xi", μ, σ). +""")) +end diff --git a/src/Features.jl b/src/Features.jl index d6efb4d..c7758c4 100644 --- a/src/Features.jl +++ b/src/Features.jl @@ -1,6 +1,7 @@ module Features include("ScalarFunctions.jl") +include("ErrorMessages.jl") import StatsBase: sample using EnsembleKalmanProcesses.ParameterDistributions, diff --git a/src/Methods.jl b/src/Methods.jl index df697f3..ee6cbc4 100644 --- a/src/Methods.jl +++ b/src/Methods.jl @@ -68,9 +68,8 @@ function RandomFeatureMethod( regularization_inverted::Bool = false, ) where {S <: AbstractString, USorMorR <: Union{<:Real, AbstractMatrix{<:Real}, UniformScaling}} - if !all([key ∈ keys(batch_sizes) for key in ["train", "test", "feature"]]) - throw(ArgumentError("batch_sizes keys must contain all of \"train\", \"test\", and \"feature\"")) - end + all(k -> haskey(batch_sizes, k), ("train", "test", "feature")) || + _throw_bad_batch_sizes(sort(collect(keys(batch_sizes)))) # ToDo store cholesky factors if isa(regularization, Real) @@ -93,7 +92,7 @@ function RandomFeatureMethod( # we work with inverted regularization matrix if !(regularization_inverted) if cond(regularization) > 10^8 - @warn "The provided regularization is poorly conditioned: κ(reg) = cond(regularization). Imprecision or SingularException during inversion may occur." + @warn "Regularization matrix is poorly conditioned (κ = $(cond(regularization)), threshold = 1e8). Inversion may lose precision or throw SingularException." end λinv = inv(λ) else @@ -616,8 +615,25 @@ end # TODO # function posterior_cov(rfm::RandomFeatureMethod, u_input, v_input) -# +# # end +## Error helpers + +@noinline function _throw_bad_batch_sizes(got_keys) + throw(ArgumentError(""" +batch_sizes dict is missing required keys. + +Expected keys: + "train", "test", "feature" + +Got keys: + $got_keys + +Suggestion: + Provide all three keys, e.g. Dict("train" => 0, "test" => 0, "feature" => 0). +""")) +end + end # module diff --git a/src/ScalarFeatures.jl b/src/ScalarFeatures.jl index 0b1b20e..e130772 100644 --- a/src/ScalarFeatures.jl +++ b/src/ScalarFeatures.jl @@ -42,14 +42,8 @@ function ScalarFeature( scalar_fun::SF; feature_parameters::Dict{S} = Dict("sigma" => 1), ) where {S <: AbstractString, SF <: ScalarFunction} - if "xi" ∉ get_name(get_parameter_distribution(feature_sampler)) - throw( - ArgumentError( - " Named parameter \"xi\" not found in names of parameter_distribution. " * - " \n Please provide the name \"xi\" to the distribution used to sample the features", - ), - ) - end + "xi" ∈ get_name(get_parameter_distribution(feature_sampler)) || + _throw_missing_xi(get_parameter_distribution(feature_sampler); where = :ScalarFeature) if "sigma" ∉ keys(feature_parameters) @info(" Required feature parameter key \"sigma\" not defined, continuing with default value \"sigma\" = 1 ") diff --git a/src/Utilities.jl b/src/Utilities.jl index 47378b3..339c779 100644 --- a/src/Utilities.jl +++ b/src/Utilities.jl @@ -147,12 +147,7 @@ function Decomposition( return Decomposition{Factor, typeof(mat), Base.return_types(cholesky, (typeof(mat),))[1]}(mat, fmat, inv(fmat)) else - throw( - ArgumentError( - "Only factorization methods \"pinv\", \"cholesky\" and \"svd\" implemented. got " * string(method), - ), - ) - + _throw_bad_decomp_method(method) end end """ @@ -236,4 +231,18 @@ end linear_solve(d::Decomposition, rhs::A; tullio_threading = true) where {A <: AbstractArray} = linear_solve(d, rhs, get_parametric_type(d), tullio_threading = tullio_threading) +## Error helpers + +@noinline function _throw_bad_decomp_method(method) + throw(ArgumentError(""" +Unrecognised matrix factorisation method. + +Expected: + "pinv", "cholesky", or "svd" + +Got: + method = $(repr(method)) +""")) +end + end diff --git a/src/VectorFeatures.jl b/src/VectorFeatures.jl index bc7ea28..62fd607 100644 --- a/src/VectorFeatures.jl +++ b/src/VectorFeatures.jl @@ -55,14 +55,8 @@ function VectorFeature( feature_parameters::Dict{S} = Dict("sigma" => 1), ) where {S <: AbstractString, SF <: ScalarFunction} - if "xi" ∉ get_name(get_parameter_distribution(feature_sampler)) - throw( - ArgumentError( - " Named parameter \"xi\" not found in names of parameter_distribution. " * - " \n Please provide the name \"xi\" to the distribution used to sample the features", - ), - ) - end + "xi" ∈ get_name(get_parameter_distribution(feature_sampler)) || + _throw_missing_xi(get_parameter_distribution(feature_sampler); where = :VectorFeature) if "sigma" ∉ keys(feature_parameters) @info(" Required feature parameter key \"sigma\" not defined, continuing with default value \"sigma\" = 1 ") diff --git a/test/Features/runtests.jl b/test/Features/runtests.jl index 42f67ba..599e9a5 100644 --- a/test/Features/runtests.jl +++ b/test/Features/runtests.jl @@ -84,11 +84,11 @@ seed = 2202 sigma_fixed_err = Dict("not sigma" => 10.0) # Error checks - @test_throws ArgumentError ScalarFeature( - n_features, - feature_sampler_err, # causes error - relu, - ) + let thrown = @test_throws ArgumentError ScalarFeature(n_features, feature_sampler_err, relu) + @test contains(thrown.value.msg, "ScalarFeature") + @test contains(thrown.value.msg, "\"xi\"") + @test contains(thrown.value.msg, "available names") + end @test_logs ( :info, " Required feature parameter key \"sigma\" not defined, continuing with default value \"sigma\" = 1 ", @@ -208,12 +208,11 @@ seed = 2202 sigma_fixed_err = Dict("not sigma" => 10.0) # Error checks - @test_throws ArgumentError VectorFeature( - n_features, - output_dim, - feature_sampler_err, # causes error - relu, - ) + let thrown = @test_throws ArgumentError VectorFeature(n_features, output_dim, feature_sampler_err, relu) + @test contains(thrown.value.msg, "VectorFeature") + @test contains(thrown.value.msg, "\"xi\"") + @test contains(thrown.value.msg, "available names") + end @test_logs ( :info, " Required feature parameter key \"sigma\" not defined, continuing with default value \"sigma\" = 1 ", diff --git a/test/Methods/runtests.jl b/test/Methods/runtests.jl index ca2f35f..b487efc 100644 --- a/test/Methods/runtests.jl +++ b/test/Methods/runtests.jl @@ -41,7 +41,10 @@ tol = 1e3 * eps() lambdamat_warn = ones(3, 3) # not pos def L = [0.5 0; 1.3 0.3] lambdamat = L * permutedims(L, (2, 1)) #pos def - @test_throws ArgumentError RandomFeatureMethod(sff, regularization = lambda, batch_sizes = batch_sizes_err) + let thrown = @test_throws ArgumentError RandomFeatureMethod(sff, regularization = lambda, batch_sizes = batch_sizes_err) + @test contains(thrown.value.msg, "missing required keys") + @test contains(thrown.value.msg, "NOT_FEATURES") + end rfm_warn = RandomFeatureMethod(sff, regularization = lambda_warn, batch_sizes = batch_sizes) @test get_regularization(rfm_warn) ≈ inv(1e12 * eps() * I) # inverted internally diff --git a/test/Utilities/runtests.jl b/test/Utilities/runtests.jl index 2ed63ce..551bf82 100644 --- a/test/Utilities/runtests.jl +++ b/test/Utilities/runtests.jl @@ -53,7 +53,10 @@ using RandomFeatures.Utilities xpinv = Decomposition(x, "pinv") @test isposdef(x) xchol = Decomposition(x, "cholesky") - @test_throws ArgumentError Decomposition(x, "qr") + let thrown = @test_throws ArgumentError Decomposition(x, "qr") + @test contains(thrown.value.msg, "\"pinv\"") + @test contains(thrown.value.msg, "\"qr\"") + end xbad = [1.0 1.0; 1.0 0.0] # not pos def xbadchol = Decomposition(xbad, "cholesky") From 26f9c3076d7fa2818dcf4bc9cd1b652fb51319f9 Mon Sep 17 00:00:00 2001 From: odunbar Date: Thu, 21 May 2026 20:20:25 -0700 Subject: [PATCH 5/5] format --- src/show.jl | 57 ++++++++++++++++++++++++++++++---------- test/Methods/runtests.jl | 6 ++++- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/show.jl b/src/show.jl index e5e81db..2855316 100644 --- a/src/show.jl +++ b/src/show.jl @@ -59,13 +59,29 @@ function Base.show(io::IO, ::MIME"text/plain", x::Features.ScalarFeature) end function Base.show(io::IO, x::Features.ScalarFeature) - print(io, "ScalarFeature (", x.n_features, " feature", - x.n_features == 1 ? "" : "s", ", ", nameof(typeof(x.scalar_function)), ")") + print( + io, + "ScalarFeature (", + x.n_features, + " feature", + x.n_features == 1 ? "" : "s", + ", ", + nameof(typeof(x.scalar_function)), + ")", + ) end function Base.summary(io::IO, x::Features.ScalarFeature) - print(io, "ScalarFeature (", x.n_features, " feature", - x.n_features == 1 ? "" : "s", ", ", nameof(typeof(x.scalar_function)), ")") + print( + io, + "ScalarFeature (", + x.n_features, + " feature", + x.n_features == 1 ? "" : "s", + ", ", + nameof(typeof(x.scalar_function)), + ")", + ) end # ── VectorFeatures ──────────────────────────────────────────────────────────── @@ -87,13 +103,29 @@ function Base.show(io::IO, ::MIME"text/plain", x::Features.VectorFeature) end function Base.show(io::IO, x::Features.VectorFeature) - print(io, "VectorFeature (", x.n_features, " features → ", x.output_dim, "D, ", - nameof(typeof(x.scalar_function)), ")") + print( + io, + "VectorFeature (", + x.n_features, + " features → ", + x.output_dim, + "D, ", + nameof(typeof(x.scalar_function)), + ")", + ) end function Base.summary(io::IO, x::Features.VectorFeature) - print(io, "VectorFeature (", x.n_features, " features → ", x.output_dim, "D, ", - nameof(typeof(x.scalar_function)), ")") + print( + io, + "VectorFeature (", + x.n_features, + " features → ", + x.output_dim, + "D, ", + nameof(typeof(x.scalar_function)), + ")", + ) end # ── RandomFeatureMethod / Fit ───────────────────────────────────────────────── @@ -107,20 +139,17 @@ function Base.show(io::IO, ::MIME"text/plain", x::Methods.RandomFeatureMethod) println(io, " feature type : ", nameof(typeof(x.random_feature))) println(io, " n_features : ", x.random_feature.n_features) println(io, " regularization: ", nameof(typeof(x.regularization))) - println(io, " batch_sizes : train=", bs["train"], - ", test=", bs["test"], ", feature=", bs["feature"]) + println(io, " batch_sizes : train=", bs["train"], ", test=", bs["test"], ", feature=", bs["feature"]) println(io, " threading : ", x.tullio_threading) end end function Base.show(io::IO, x::Methods.RandomFeatureMethod) - print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), - ", n=", x.random_feature.n_features, ")") + print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), ", n=", x.random_feature.n_features, ")") end function Base.summary(io::IO, x::Methods.RandomFeatureMethod) - print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), - ", n=", x.random_feature.n_features, ")") + print(io, "RandomFeatureMethod (", nameof(typeof(x.random_feature)), ", n=", x.random_feature.n_features, ")") end function Base.show(io::IO, ::MIME"text/plain", x::Methods.Fit) diff --git a/test/Methods/runtests.jl b/test/Methods/runtests.jl index b487efc..1b0f734 100644 --- a/test/Methods/runtests.jl +++ b/test/Methods/runtests.jl @@ -41,7 +41,11 @@ tol = 1e3 * eps() lambdamat_warn = ones(3, 3) # not pos def L = [0.5 0; 1.3 0.3] lambdamat = L * permutedims(L, (2, 1)) #pos def - let thrown = @test_throws ArgumentError RandomFeatureMethod(sff, regularization = lambda, batch_sizes = batch_sizes_err) + let thrown = @test_throws ArgumentError RandomFeatureMethod( + sff, + regularization = lambda, + batch_sizes = batch_sizes_err, + ) @test contains(thrown.value.msg, "missing required keys") @test contains(thrown.value.msg, "NOT_FEATURES") end