Skip to content

Fix docstring warnings and remove warnonly from makedocs (#378)#909

Open
Kk-304 wants to merge 1 commit intoSciML:mainfrom
Kk-304:main
Open

Fix docstring warnings and remove warnonly from makedocs (#378)#909
Kk-304 wants to merge 1 commit intoSciML:mainfrom
Kk-304:main

Conversation

@Kk-304
Copy link

@Kk-304 Kk-304 commented Mar 1, 2026

Fix docstring warnings and remove warnonly from makedocs (#378)

  • Remove warnonly = [:docs_block, :missing_docs] from docs/make.jl
    so documentation errors are no longer silently ignored.
  • Remove SciMLBase from modules list to avoid 500+ irrelevant
    missing_docs warnings from the dependency.
  • Add forwarding docstring for LinearProblem (defined in SciMLBase,
    reexported by LinearSolve).
  • Add missing docstring for LDLtFactorization.
  • Fix defaultalg docstring attachment (comments between docstring and
    function prevented Julia from associating them).
  • Improve defaultalg_adjoint_eval docstring.
  • Convert SciMLBase.solve! dispatch docstring to a comment to avoid
    cross-module missing_docs error.
  • Add 13 missing public solver entries to solvers.md:
    GenericFactorization, ButterflyFactorization, RF32MixedLUFactorization,
    LDLtFactorization, KrylovJL_MINARES, KrylovJL_FGMRES,
    OpenBLAS32MixedLUFactorization, PanuaPardisoFactorize,
    PanuaPardisoIterate, CudaOffloadFactorization, BLISLUFactorization,
    AlgebraicMultigridJL, PETScAlgorithm, LinearSolveAdjoint.
  • Remove duplicate @docs entries from internal_api.md and add missing
    internal API docs (needs_square_A, defaultalg_adjoint_eval,
    BlasOperationInfo, interpret_blas_code, blas_info_msg,
    _format_blas_context, _sym_givens).

Copilot AI review requested due to automatic review settings March 1, 2026 20:23
Comment on lines +99 to +102
# Attach LinearProblem docstring to LinearSolve module for Documenter.jl
# (LinearProblem is defined in SciMLBase but reexported here)
@doc """
LinearProblem(A, b; u0 = nothing, p = nothing)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this is piracy

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes Documenter “warnonly” handling and fixes/expands documentation so docstring and missing-doc warnings become actionable during docs builds.

Changes:

  • Made documentation builds strict by removing warnonly and trimming modules to avoid dependency noise.
  • Added/updated docstrings (e.g., LDLtFactorization, LinearProblem, defaultalg_adjoint_eval) and converted a problematic cross-module docstring to a comment.
  • Expanded solver and internal API docs pages with missing @docs entries.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/factorization.jl Adds a public docstring for LDLtFactorization.
src/default.jl Fixes docstring attachment issues and improves internal documentation for default solver/adjoint dispatch.
src/LinearSolve.jl Adds a forwarding docstring for LinearProblem in this module for Documenter visibility.
docs/src/solvers/solvers.md Adds missing @docs entries for many public solver algorithms and new doc sections.
docs/src/assets/Project.toml Introduces a docs build environment with explicit deps/compat.
docs/src/advanced/internal_api.md Cleans up internal docs and adds missing internal API entries.
docs/make.jl Makes docs stricter (removes warnonly) and avoids pulling SciMLBase docs into modules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +501 to +511
"""
LDLtFactorization(shift = 0.0, perm = nothing)

A wrapper around `LinearAlgebra.ldlt!` for `LDLt` factorization of symmetric
(or Hermitian) positive semi-definite tridiagonal matrices (e.g. `SymTridiagonal`).

## Keyword Arguments

- `shift`: Diagonal shift applied before factoring. Defaults to `0.0`.
- `perm`: Optional permutation vector. Defaults to `nothing`.

Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring documents a keyword-style constructor (LDLtFactorization(shift = ..., perm = ...)), but the shown definition is a plain struct (no Base.@kwdef / explicit keyword outer constructor visible here). If callers can’t actually use keyword construction, this docstring will be misleading and (now that docs are strict) may cause doctest/examples drift. Consider either (a) adding an explicit keyword constructor (or Base.@kwdef if appropriate for this codebase), or (b) updating the signature + “Keyword Arguments” section to match the actual constructor(s).

Copilot uses AI. Check for mistakes.
Comment on lines 522 to 524
struct LDLtFactorization{T} <: AbstractDenseFactorization
shift::Float64
perm::T
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring documents a keyword-style constructor (LDLtFactorization(shift = ..., perm = ...)), but the shown definition is a plain struct (no Base.@kwdef / explicit keyword outer constructor visible here). If callers can’t actually use keyword construction, this docstring will be misleading and (now that docs are strict) may cause doctest/examples drift. Consider either (a) adding an explicit keyword constructor (or Base.@kwdef if appropriate for this codebase), or (b) updating the signature + “Keyword Arguments” section to match the actual constructor(s).

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +115
@doc """
LinearProblem(A, b; u0 = nothing, p = nothing)
LinearProblem{iip}(A, b; u0 = nothing, p = nothing)

Define a linear system problem ``Au = b``.

## Arguments

- `A`: The coefficient matrix or linear operator. Can be a dense matrix, sparse matrix,
or any `AbstractSciMLOperator`.
- `b`: The right-hand side vector.
- `u0`: (optional) Initial guess for iterative solvers. Defaults to `nothing`.
- `p`: (optional) Parameters for the problem. Defaults to `nothing`.

## Example
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a new external link while docs/make.jl has linkcheck = true. External link checks can be flaky in CI (network/DNS/transient outages), which becomes more painful now that doc warnings are no longer ignored. Consider adding https://docs.sciml.ai/SciMLBase/stable/ to linkcheck_ignore, or switching to a non-linkchecked reference pattern if the project prefers strict linkchecking.

Copilot uses AI. Check for mistakes.
Comment on lines +375 to +379
### BLIS

!!! note

Using this solver requires adding the packages blis_jll and LAPACK_jll, i.e. `using blis_jll, LAPACK_jll`
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These new @docs blocks reference solvers that (per the notes) require optional packages/JLLs. With warnonly removed, docs will now hard-fail if these symbols aren’t defined/loaded during documentation build (common when solver types are provided via package extensions that only activate when the dependency is in the docs environment). Consider ensuring the docs environment includes the needed optional deps (e.g., blis_jll, LAPACK_jll, AlgebraicMultigrid, PETSc, possibly SparseArrays where relevant), or conditionally including these @docs blocks only when the dependency is available.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants