Minor update to RichardsonLinearSolver#106
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates RichardsonLinearSolver to accept a broader set of vector-like relaxation parameters by widening the stored type from Vector{Float64} to AbstractVector{Float64}, and documents the change in the changelog.
Changes:
- Widened the
ωfield type to acceptAbstractVector{Float64}in addition toFloat64. - Added an “Unreleased” NEWS entry describing the update.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/LinearSolvers/RichardsonLinearSolvers.jl |
Broadens the accepted relaxation parameter container type (Vector → AbstractVector). |
NEWS.md |
Adds an Unreleased changelog entry for the Richardson relaxation parameter update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| struct RichardsonLinearSolver<:Gridap.Algebra.LinearSolver | ||
| ω::Union{Vector{Float64},Float64} | ||
| ω::Union{AbstractVector{Float64},Float64} |
There was a problem hiding this comment.
Using an abstract type (AbstractVector{Float64}) as a struct field can introduce dynamic dispatch/type-instability when ω is accessed inside hot loops. Consider making RichardsonLinearSolver parametric over the relaxation type (e.g., RichardsonLinearSolver{W} with ω::W and a constructor that restricts W to Float64 or AbstractVector{Float64}) to keep instances concretely typed while still accepting any AbstractVector implementation.
| """ | ||
| struct RichardsonLinearSolver<:Gridap.Algebra.LinearSolver | ||
| ω::Union{Vector{Float64},Float64} | ||
| ω::Union{AbstractVector{Float64},Float64} |
There was a problem hiding this comment.
The docstring above this struct still says the relaxation parameter can be a Vector{Float64}; with this change it should be updated to reflect AbstractVector{Float64} (or whatever types are intended) so user-facing docs match the actual accepted types.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #106 +/- ##
=======================================
Coverage 66.55% 66.55%
=======================================
Files 62 62
Lines 4072 4072
=======================================
Hits 2710 2710
Misses 1362 1362
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Updating the struct to handle a more wide range of relaxation parameters.