Fix bounded Tikhonov solver, regularization issues, and improve validation - #16
Open
NilsWildt wants to merge 17 commits into
Open
Fix bounded Tikhonov solver, regularization issues, and improve validation#16NilsWildt wants to merge 17 commits into
NilsWildt wants to merge 17 commits into
Conversation
added 17 commits
August 25, 2026 08:57
The bounded solver fed a scalar objective and gradient into LeastSquaresOptim's residual/Jacobian interface, so Dogleg converged to a point worse than its starting value. Replace it with Optim.Fminbox on the correct objective and update the affected regression values.
The constructor passed L⁺ₐ and L⁺ in the wrong order, so Ψ.L⁺ held the A-weighted generalized inverse and Ψ.L⁺ₐ held the plain pseudoinverse. It worked only because to_general_form happened to read Ψ.L⁺.
Negative Γ order recursed infinitely; degenerate L dimensions were silently accepted. Throw clear ArgumentError/DimensionMismatch instead.
Drop the unused x̄ argument and the scalar [1]/broadcast no-ops.
Use p.x instead of getfield and throw ArgumentError on unknown methods.
Cover input validation (Γ, setupRegularizationProblem), the L⁺/L⁺ₐ field values, bounded-solve correctness, the designmatrix shortcut, the non-bounds λ search, and type stability. Raises coverage from 97.0% to 98.5%.
ĀĀ + λ²I is positive definite for λ > 0, so the cholesky fast path always succeeds and the fallback to \ was dead code. Use cholesky! directly and fail fast.
InverseMethod is a sealed @DaTa type with all eight variants matched, so the catch-all clause could never fire.
Concrete field types remove dynamic dispatch in designmatrix and forwardmodel, which now pass @inferred. Drop the abstract return annotations that masked the inferred return type.
The unparameterized F̄::SVD field was abstract, so F.U/F.S access in gcv_svd was dynamically dispatched and boxed ~6500 Float64 scalars per call. Parameterizing by the concrete SVD type makes gcv_svd type-stable (6574 -> 3 allocations, 4.5x faster). The cached Āᵀ field was never read, so drop it.
The comprehension evaluated x(i) once per (i,j) pair, so each of the n basis vectors was rebuilt m times. Precompute them once, cutting allocation ~19x and runtime ~5x for a 100x100 domain.
- Incorporate CairoMakie library for plotting - Create a Figure and an Axis - Plot the true distribution, estimated distribution, and add labels and �[K axis - Update `smoothing.jl`: - Use CairoMakie for plotting - Create a Figure and an Axis - Plot the original data, regularization tools, and direct method, add la�[2D�[K labels and axis
… point�[5D�[K points - Added `scatterlines!(ax, Dp, xλ, label = "Estimated")` to plot estimated �[K data points
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Me + LLM updated the repo, fixed jetls found type instabilities, added tests, made sure we would pass all tests, fixed bugs, changed Plotting to CairoMakie.jl.